-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support for multiple paths in in source configuration #230
Conversation
node/declaration.ts
Outdated
declarations.push({ ...declaration, ...config, path }) | ||
}) | ||
// if empty path array with cache set, add declaration with cache | ||
else if (config.cache) declarations.push({ ...declaration, cache: config.cache }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand why we're addressing the cache
property specifically here. What happens if we add more configuration properties? Will they have to be added to these conditions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eduardoboucas I made a change that will include any type of config, I think it had to do with the overall conditional in the function. Let me know if it doesn't make sense?
c1ae79c
to
cbf007d
Compare
b84e1a7
to
e401ff3
Compare
test/fixtures/with_config/.netlify/edge-functions/framework-func1.ts
Outdated
Show resolved
Hide resolved
@eduardoboucas sorry all my comments weren't sent because I did not review correctly :p |
Co-authored-by: Eduardo Bouças <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ Awesome work, Jackie and Karin! ❤️
…edge-bundler#230) * feat: support for multiple paths in in source configuration * fix: use array for isc path properties * fix: support both array and string path properties * fix: fix linting * fix: have a string usecase in declaration tests by transforming json array to string * fix: remove file-level eslint-disable-max-depth * fix: change conditional for cache config and path length * fix: change conditional for adding all different types of config * fix: remove fallback to empty object * fix: change comment * fix: undo changing config fixtures to use an array as path * fix: update comments * Update node/declaration.ts Co-authored-by: Eduardo Bouças <[email protected]> * fix: change mutation of config.path * fix: formatting Co-authored-by: khen <[email protected]> Co-authored-by: Eduardo Bouças <[email protected]>
🎉 Thanks for sending this pull request! 🎉
https://github.com/netlify/pod-compute/issues/295
This PR adds support for multiple paths in ISC. It allows for path to be either a string or array of strings.
Checklist
Please add a
x
inside each checkbox:A picture of a cute animal (not mandatory but encouraged)
