Repository navigation
fix: tarball bundling - import assertion replacement - #6928
Conversation
|
This pull request adds or modifies JavaScript ( |
| .forEach((node) => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access | ||
| const statement = source.slice((node as any).source.end, node.end) | ||
| const newStatement = statement.replace('assert', 'with') |
There was a problem hiding this comment.
What example statement looks like in those AST nodes?
Will this be something like assert { type: 'json' }?
If so I think this is fine for what deno actually supports - because it's just type property with values being just "json" or "bytes" and "text" (some of them seems to need some flag to enable) (ref https://github.com/denoland/deno/blob/8871ade7aa09eb907a04ee61a427f5ba14c54014/runtime/worker.rs#L97-L114 )
If this would be entire import/export statement - like say import assert from "./assert.json" assert { type: "json" } this would replace just first assert text occurrence which would not be correct change. Can we add this weird import statement as one of test inputs?
There was a problem hiding this comment.
Yes, it will be assert { type: 'json' }
I added import assert from './assert.json' with { type: 'json' }; and console.assert(true, 'should be true'); to cover the case of "over capturing"
Summary
Part of FRB-2115
Note: When the syntax is unrecognized / fails being parsed into an AST, the input code is returned as is (e.g. when using JSX syntax)
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)