fix(esm): dont bail for preflight requests#39237
fix(esm): dont bail for preflight requests#39237Skn0tt wants to merge 1 commit intomicrosoft:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| const code = fs.readFileSync(filename, 'utf-8'); | ||
| const transformed = transformHook(code, filename, moduleUrl); |
There was a problem hiding this comment.
I'd skip this for preflights that we want to bail on, e.g. from node_modules. Since we are not supposed to transform them, we don't need to prepopulate source mapes.
I'd make a custom defaultLoad function that would return void 0 for preflights or call the actual defaultLoad for non-preflights.
There was a problem hiding this comment.
I didn't like the custom defaultLoad function, but I found a different way of writing it that skips the fs ops and transform for preflights, and looks pretty readable.
This comment has been minimized.
This comment has been minimized.
Skn0tt
force-pushed
the
dont-bail-for-preflight
branch
from
7fafc93 to
be4648f
Compare
Test results for "tests 1"14 failed 2 flaky38514 passed, 843 skipped Merge workflow run. |
Test results for "MCP"5 failed 4787 passed, 135 skipped Merge workflow run. |
| moduleUrl = moduleUrl.replace(esmPreflightExtension, ''); | ||
| const filename = url.fileURLToPath(moduleUrl); | ||
| const format = kSupportedFormats.get(context.format) || (fileIsModule(filename) ? 'module' : 'commonjs'); | ||
| if (isPreflight) |
There was a problem hiding this comment.
This always skips transformHook which is the sole purpose of having a preflight.
2 participants
Closes #39172.
Interestingly, the test without the fix immediately leads to the
Error: duplicate test title "another test", first declared in example.spec.ts.esm.preflight:13bug, so we have a good repro even without tsx. It seems to affect any loader.