Closed guybedford closed 4 years ago
Fixes #355.
Instead of trying to automatically detect optional requires here I've instead added a new isMissing
hook which is only applied for require statements that are found within try-catch statements.
This hook is given the id and parent Id and can then return true or false if the require should be "inlined as a not found require" or not.
That also avoids polluting the default case since this feature is now fully opt-in.
This is quite an important one
Hugely excited for this!
It looks to me like the resolution is done at compile time when running roll-up. Is it possible to do it at runtime instead?
Our use case: we're building a chart library called Chart.js and want to give it peerDependencies
on luxon and moment. If the user wants to plot a time chart they must supply one of these two time libraries. We won't know when compiling Chart.js if the user will be supplying one of these libraries. Instead, it is known only at runtime
Is it still planned to merge this.
Here's a plugin for optional requires that's used by Chart.js: https://github.com/chartjs/Chart.js/blob/master/rollup.plugins.js
Here's a plugin for optional requires that's used by Chart.js: https://github.com/chartjs/Chart.js/blob/master/rollup.plugins.js
Hmm it is not exactly what I am looking for. If I know which deps are optional I can do that, however I am building a whole projects where multiple dependencies, are doing require in try catches. I could probably find all cases for my project, but some libs are actually behaving differently depending on wether a module could be loaded, so the exception has to be thrown where the original require was.
To give an example in one case a module is shipping an optional native NodeJS addon, which cannot be bundled (easily), therefore if the require fails it uses a fallback pure JS implementation.
This PR really seems like the ideal solution. I thought about creating my own plugin, which would pre or posttransform the code, but it is much "uglier" and more complicated than doing it inside plugin-commonjs.
@guybedford any chance you're up for resolving these conflicts?
If anyone else would like to fork from guy's fork and complete the conflict resolutions, we'd welcome that as well.
We're getting ready to move the plugin to a new home at https://github.com/rollup/plugins and we'd like to see this get merged before that point. If it can't, we'll welcome a PR for the same changes in the new repo.
@shellscape as per the other issue don't have any pressing need to see this merged anymore here. Perhaps an issue in the new repo specifically about supporting try-catch optional requires can link to this PR as a feature as the PR may provide some base to work from for any future contributors wanting this feature.
@guybedford great, thank you. I will do exactly that.
When a require does not resolve that exists within a try-catch statement, this PR supports that being emitted as an inlined "module not found" error.
This is useful for example with try-catch statements in optional dependencies.
In order to run the async resolver within the transform function to determine if these optional dependencies resolve or not, the entire transform function has been made async, which makes the diff look like more than there is unfortunately.
I did try moving to async/await but I'm not sure the codebase is ready for that? Happy to as well if we can though.