Pass filename to importInterop method#14456
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51671/ |
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
Thank you! I admit I completely forgot about our slack discussion, but still as soon as I saw this PR I thought "this is a good idea!".
Can you also add a test for what happens when no filename is passed to Babel, and if we need to update the type annotation to string | undefined?
| ? mjsStrictNamespace | ||
| : strictNamespace, | ||
| noIncompleteNsImportDetection, | ||
| filename: this.file.opts.filename, |
There was a problem hiding this comment.
We also use rewriteModuleStatementsAndPrepareHeader in the amd and umd plugins. I don't remember if they all support the importInterop option, but if they do they should have the same behavior.
There was a problem hiding this comment.
Yeah, that's covered by what I already have in the diff, right? Or am I missing something?
Ah yes, good point! |
|
@nicolo-ribaudo thanks for your review so far! What are the next steps on this? |
JLHwung
left a comment
There was a problem hiding this comment.
Can you open a docs PR to babel/website?
https://babeljs.io/docs/en/babel-plugin-transform-modules-commonjs#importinterop and the amd/umd accordingly.
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
Thanks! We will merge this PR when we are ready for the next minor.
|
Woooo thanks for adding more tests and finding a bug. 😄 |
Done! babel/website#2639. |
Fixes #1, Fixes #2babel/plugin-transform-modules-commonjs's
importInteropfunction signature is currently:(specifier: string) =>. This allows the function to implement heuristic-based decisions, like "if it starts with./, then use strategybabel, otherwisenode.".I would like to implement an
importInteropfunction in my project where the build process actually inspects the file being imported, then picks the right strategy based on that file's contents. I can't do this from thespecifieralone, because I need to know where to resolve from. For that, we must also pass thefilenameof the file doing the import.Slack context.