Fix passing filename from @babel/core to @babel/parser
#15911
Conversation
nicolo-ribaudo
added
the
PR: Bug Fix 🐛
nicolo-ribaudo
force-pushed
the
fix-passing-filename-to-parser
branch
from
75dd091
to
3fd6f52
Compare
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55322/ |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/55316/ |
packages/babel-core/test/api.js
Outdated
| }); | ||
|
|
||
| expect(result.map.sources).toEqual(["path.js", "file/path.js"]); |
There was a problem hiding this comment.
I believe the old one here was wrong, but:
- is the new one correct?
- having both is definitely wrong
nicolo-ribaudo
force-pushed
the
fix-passing-filename-to-parser
branch
from
95f0ba9
to
82cc0e0
Compare
| }); | ||
|
|
||
| expect(result.map.sources).toEqual(["file/path.js"]); |
There was a problem hiding this comment.
I'm not sure if we should consider this PR as breaking because of this change, or as a bugfix. I'm not even sure if this is correct, but it's probably the best we can do given that we do not know the final location of the source map on disk.
| @@ -0,0 +1,339 @@ | |||
| { | |||
There was a problem hiding this comment.
This file is a copy of output.json, but every loc has the filename property.
| @@ -30,7 +39,10 @@ export default function normalizeOptions(config: ResolvedConfig): {} { | |||
| sourceType: | |||
| path.extname(filenameRelative) === ".mjs" ? "module" : sourceType, | |||
|
|
|||
| sourceFileName: filename, | |||
There was a problem hiding this comment.
The plugin babel-plugin-bundled-import-meta is relying on this option to get the complete input filename,
https://github.com/cfware/babel-plugin-bundled-import-meta/blob/master/index.js#L67
We reverted the change in #13732, we will probably have to defer this PR to Babel 8.
There was a problem hiding this comment.
We could keep passing sourceFileName to avoid breaking plugins that rely on it -- given that it's ignored by @babel/parser, there is no harm in doing so.
There was a problem hiding this comment.
If we start to pass both base filename as sourceFilename for source map correctness, and complete filename as sourceFileName for legacy compatibility, that would be indeed confusing. The parser docs should then clarify that sourceFileName is deprecated and we offer a transition strategy, maybe a new property in the File class offered by @babel/core?
Fixes #1, Fixes #2This is blocking #9509. All the source map tests are now broken, and I have to figure out what is the correct behavior.