Skip to content

Commit 5cdd50b

Browse files
privatenumberprivatenumber
committedMay 16, 2024·
perf: skip parsing if import( is not found in minified code
·
v4.19.3v4.10.3
1 parent ad970ac commit 5cdd50b

File tree

4 files changed

+63
-44
lines changed

4 files changed

+63
-44
lines changed
 

‎src/utils/es-module-lexer.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@ init.then(() => {
88
wasmParserInitialized = true;
99
});
1010

11-
export const parseEsm = (code: string) => (
11+
export const parseEsm = (
12+
code: string,
13+
filename: string,
14+
) => (
1215
wasmParserInitialized
13-
? parseWasm(code)
14-
: parseJs(code)
16+
? parseWasm(code, filename)
17+
: parseJs(code, filename)
1518
);

‎src/utils/transform/index.ts

+6-8
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,17 @@ export const transformSync = (
6868
filePath,
6969
code,
7070
[
71-
// eslint-disable-next-line @typescript-eslint/no-shadow
72-
(_filePath, code) => {
71+
(_filePath, _code) => {
7372
const patchResult = patchOptions(esbuildOptions);
7473
let result;
7574
try {
76-
result = esbuildTransformSync(code, esbuildOptions);
75+
result = esbuildTransformSync(_code, esbuildOptions);
7776
} catch (error) {
7877
throw formatEsbuildError(error as TransformFailure);
7978
}
8079
return patchResult(result);
8180
},
82-
transformDynamicImport,
81+
(_filePath, _code) => transformDynamicImport(_filePath, _code, true),
8382
],
8483
);
8584

@@ -115,18 +114,17 @@ export const transform = async (
115114
filePath,
116115
code,
117116
[
118-
// eslint-disable-next-line @typescript-eslint/no-shadow
119-
async (_filePath, code) => {
117+
async (_filePath, _code) => {
120118
const patchResult = patchOptions(esbuildOptions);
121119
let result;
122120
try {
123-
result = await esbuildTransform(code, esbuildOptions);
121+
result = await esbuildTransform(_code, esbuildOptions);
124122
} catch (error) {
125123
throw formatEsbuildError(error as TransformFailure);
126124
}
127125
return patchResult(result);
128126
},
129-
transformDynamicImport,
127+
(_filePath, _code) => transformDynamicImport(_filePath, _code, true),
130128
],
131129
);
132130

‎src/utils/transform/transform-dynamic-import.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,22 @@ const handleDynamicImport = `.then(${toEsmFunctionString})`;
2525
export const transformDynamicImport = (
2626
filePath: string,
2727
code: string,
28+
isMinified?: boolean,
2829
) => {
2930
// Naive check (regex is too slow)
30-
if (!code.includes('import')) {
31+
if (isMinified) {
32+
// If minified, we can safely check for "import(" to avoid parsing
33+
if (!code.includes('import(')) {
34+
return;
35+
}
36+
} else if (!code.includes('import')) {
37+
// This is a bit more expensive as we end up parsing even if import statements are detected
3138
return;
3239
}
3340

34-
const dynamicImports = parseEsm(code)[0].filter(maybeDynamic => maybeDynamic.d > -1);
35-
41+
// Passing in the filePath improves Parsing Error message
42+
const parsed = parseEsm(code, filePath);
43+
const dynamicImports = parsed[0].filter(maybeDynamic => maybeDynamic.d > -1);
3644
if (dynamicImports.length === 0) {
3745
return;
3846
}

‎tests/specs/smoke.ts

+40-30
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ const sourcemap = {
8282
// Adding the dynamic import helps test the import transformation's source map
8383
test: (
8484
extension: string,
85-
) => `import('node:fs');\nconst { stack } = new Error(); const searchString = 'index.${extension}:SOURCEMAP_LINE'; assert(stack.includes(searchString), \`Expected \${searchString} in stack: \${stack}\`)`,
85+
) => `import ('node:fs');\nconst { stack } = new Error(); const searchString = 'index.${extension}:SOURCEMAP_LINE'; assert(stack.includes(searchString), \`Expected \${searchString} in stack: \${stack}\`)`,
8686
tag: (
8787
strings: TemplateStringsArray,
8888
...values: string[]
@@ -108,11 +108,21 @@ const files = {
108108
assert(${cjsContextCheck}, 'Should have CJS context');
109109
${preserveName}
110110
${sourcemap.test('cjs')}
111+
112+
// Assert __esModule is unwrapped
113+
import ('../ts/index.ts').then((m) => assert(
114+
!(typeof m.default === 'object' && ('default' in m.default)),
115+
));
111116
exports.named = 'named';
112117
`,
113118

114119
'mjs/index.mjs': `
120+
import assert from 'assert';
115121
export const mjsHasCjsContext = ${cjsContextCheck};
122+
123+
import ('pkg-commonjs').then((m) => assert(
124+
!(typeof m.default === 'object' && ('default' in m.default)),
125+
));
116126
`,
117127

118128
'ts/index.ts': sourcemap.tag`
@@ -255,9 +265,9 @@ const files = {
255265
[() => ${jsxCheck}, 'React is not defined'],
256266
257267
// These should throw unless allowJs is set
258-
// [() => import('prefix/file'), "Cannot find package 'prefix'"],
259-
// [() => import('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
260-
// [() => import('file'), "Cannot find package 'file'"],
268+
// [() => import ('prefix/file'), "Cannot find package 'prefix'"],
269+
// [() => import ('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
270+
// [() => import ('file'), "Cannot find package 'file'"],
261271
);
262272
`,
263273

@@ -271,9 +281,9 @@ const files = {
271281
'index.mjs': `
272282
import { expectErrors } from '../../../expect-errors';
273283
expectErrors(
274-
[() => import('prefix/file'), "Cannot find package 'prefix'"],
275-
[() => import('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
276-
[() => import('file'), "Cannot find package 'file'"],
284+
[() => import ('prefix/file'), "Cannot find package 'prefix'"],
285+
[() => import ('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
286+
[() => import ('file'), "Cannot find package 'file'"],
277287
);
278288
`,
279289
'index.cjs': `
@@ -366,7 +376,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
366376
import './js/';
367377
368378
// No double .default.default in Dynamic Import
369-
import('./js/index.js').then(m => {
379+
import/* comment */('./js/index.js').then(m => {
370380
if (typeof m.default === 'object') {
371381
assert(
372382
!('default' in m.default),
@@ -375,7 +385,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
375385
}
376386
});
377387
378-
const importWorksInEval = async () => await import('./js/index.js');
388+
const importWorksInEval = async () => await import ('./js/index.js');
379389
(0, eval)(importWorksInEval.toString())();
380390
381391
// .json
@@ -386,8 +396,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
386396
// .cjs
387397
import * as cjs from './cjs/index.cjs';
388398
expectErrors(
389-
[() => import('./cjs/index'), 'Cannot find module'],
390-
[() => import('./cjs/'), 'Cannot find module'],
399+
[() => import ('./cjs/index'), 'Cannot find module'],
400+
[() => import ('./cjs/'), 'Cannot find module'],
391401
${
392402
isCommonJs
393403
? `
@@ -401,8 +411,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
401411
// .mjs
402412
import * as mjs from './mjs/index.mjs';
403413
expectErrors(
404-
[() => import('./mjs/index'), 'Cannot find module'],
405-
[() => import('./mjs/'), 'Cannot find module'],
414+
[() => import ('./mjs/index'), 'Cannot find module'],
415+
[() => import ('./mjs/'), 'Cannot find module'],
406416
${
407417
isCommonJs
408418
? `
@@ -418,8 +428,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
418428
419429
// Unsupported files
420430
expectErrors(
421-
[() => import('./file.txt'), 'Unknown file extension'],
422-
[() => import(${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
431+
[() => import ('./file.txt'), 'Unknown file extension'],
432+
[() => import (${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
423433
${
424434
isCommonJs
425435
? `
@@ -433,7 +443,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
433443
? '[() => require(\'./broken-syntax\'), \'Transform failed\'],'
434444
: ''
435445
}
436-
[() => import('./broken-syntax'), 'Transform failed'],
446+
[() => import ('./broken-syntax'), 'Transform failed'],
437447
);
438448
439449
console.log(JSON.stringify({
@@ -510,7 +520,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
510520
)};
511521
512522
// No double .default.default in Dynamic Import
513-
import('./js/index.js').then(m => {
523+
import/* comment */('./js/index.js').then(m => {
514524
if (typeof m.default === 'object') {
515525
assert(
516526
!('default' in m.default),
@@ -527,8 +537,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
527537
// .cjs
528538
import * as cjs from './cjs/index.cjs';
529539
expectErrors(
530-
[() => import('./cjs/index'), 'Cannot find module'],
531-
[() => import('./cjs/'), 'Cannot find module'],
540+
[() => import ('./cjs/index'), 'Cannot find module'],
541+
[() => import ('./cjs/'), 'Cannot find module'],
532542
${
533543
isCommonJs
534544
? `
@@ -542,8 +552,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
542552
// .mjs
543553
import * as mjs from './mjs/index.mjs';
544554
expectErrors(
545-
[() => import('./mjs/index'), 'Cannot find module'],
546-
[() => import('./mjs/'), 'Cannot find module'],
555+
[() => import ('./mjs/index'), 'Cannot find module'],
556+
[() => import ('./mjs/'), 'Cannot find module'],
547557
${
548558
isCommonJs
549559
? `
@@ -578,9 +588,9 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
578588
import './cts/index.cjs';
579589
expectErrors(
580590
// TODO:
581-
// [() => import('./cts/index.cts'), 'Cannot find module'],
582-
[() => import('./cts/index'), 'Cannot find module'],
583-
[() => import('./cts/'), 'Cannot find module'],
591+
// [() => import ('./cts/index.cts'), 'Cannot find module'],
592+
[() => import ('./cts/index'), 'Cannot find module'],
593+
[() => import ('./cts/'), 'Cannot find module'],
584594
${
585595
isCommonJs
586596
? `
@@ -596,9 +606,9 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
596606
import './mts/index.mjs';
597607
expectErrors(
598608
// TODO:
599-
// [() => import('./mts/index.mts'), 'Cannot find module'],
600-
[() => import('./mts/index'), 'Cannot find module'],
601-
[() => import('./mts/'), 'Cannot find module'],
609+
// [() => import ('./mts/index.mts'), 'Cannot find module'],
610+
[() => import ('./mts/index'), 'Cannot find module'],
611+
[() => import ('./mts/'), 'Cannot find module'],
602612
${
603613
isCommonJs
604614
? `
@@ -612,8 +622,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
612622
613623
// Unsupported files
614624
expectErrors(
615-
[() => import('./file.txt'), 'Unknown file extension'],
616-
[() => import(${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
625+
[() => import ('./file.txt'), 'Unknown file extension'],
626+
[() => import (${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
617627
${
618628
isCommonJs
619629
? `
@@ -627,7 +637,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
627637
? '[() => require(\'./broken-syntax\'), \'Transform failed\'],'
628638
: ''
629639
}
630-
[() => import('./broken-syntax'), 'Transform failed'],
640+
[() => import ('./broken-syntax'), 'Transform failed'],
631641
);
632642
633643
console.log(JSON.stringify({

0 commit comments

Comments
 (0)
Please sign in to comment.