Skip to content

Commit

Permalink
perf: skip parsing if import( is not found in minified code
Browse files Browse the repository at this point in the history
  • Loading branch information
@privatenumber
privatenumber committed May 16, 2024
1 parent ad970ac commit 5cdd50b
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 44 deletions.
9 changes: 6 additions & 3 deletions src/utils/es-module-lexer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ init.then(() => {
wasmParserInitialized = true;
});

export const parseEsm = (code: string) => (
export const parseEsm = (
code: string,
filename: string,
) => (
wasmParserInitialized
? parseWasm(code)
: parseJs(code)
? parseWasm(code, filename)
: parseJs(code, filename)
);
14 changes: 6 additions & 8 deletions src/utils/transform/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,17 @@ export const transformSync = (
filePath,
code,
[
// eslint-disable-next-line @typescript-eslint/no-shadow
(_filePath, code) => {
(_filePath, _code) => {
const patchResult = patchOptions(esbuildOptions);
let result;
try {
result = esbuildTransformSync(code, esbuildOptions);
result = esbuildTransformSync(_code, esbuildOptions);
} catch (error) {
throw formatEsbuildError(error as TransformFailure);
}
return patchResult(result);
},
transformDynamicImport,
(_filePath, _code) => transformDynamicImport(_filePath, _code, true),
],
);

Expand Down Expand Up @@ -115,18 +114,17 @@ export const transform = async (
filePath,
code,
[
// eslint-disable-next-line @typescript-eslint/no-shadow
async (_filePath, code) => {
async (_filePath, _code) => {
const patchResult = patchOptions(esbuildOptions);
let result;
try {
result = await esbuildTransform(code, esbuildOptions);
result = await esbuildTransform(_code, esbuildOptions);
} catch (error) {
throw formatEsbuildError(error as TransformFailure);
}
return patchResult(result);
},
transformDynamicImport,
(_filePath, _code) => transformDynamicImport(_filePath, _code, true),
],
);

Expand Down
14 changes: 11 additions & 3 deletions src/utils/transform/transform-dynamic-import.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,22 @@ const handleDynamicImport = `.then(${toEsmFunctionString})`;
export const transformDynamicImport = (
filePath: string,
code: string,
isMinified?: boolean,
) => {
// Naive check (regex is too slow)
if (!code.includes('import')) {
if (isMinified) {
// If minified, we can safely check for "import(" to avoid parsing
if (!code.includes('import(')) {
return;
}
} else if (!code.includes('import')) {
// This is a bit more expensive as we end up parsing even if import statements are detected
return;
}

const dynamicImports = parseEsm(code)[0].filter(maybeDynamic => maybeDynamic.d > -1);

// Passing in the filePath improves Parsing Error message
const parsed = parseEsm(code, filePath);
const dynamicImports = parsed[0].filter(maybeDynamic => maybeDynamic.d > -1);
if (dynamicImports.length === 0) {
return;
}
Expand Down
70 changes: 40 additions & 30 deletions tests/specs/smoke.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const sourcemap = {
// Adding the dynamic import helps test the import transformation's source map
test: (
extension: string,
) => `import('node:fs');\nconst { stack } = new Error(); const searchString = 'index.${extension}:SOURCEMAP_LINE'; assert(stack.includes(searchString), \`Expected \${searchString} in stack: \${stack}\`)`,
) => `import ('node:fs');\nconst { stack } = new Error(); const searchString = 'index.${extension}:SOURCEMAP_LINE'; assert(stack.includes(searchString), \`Expected \${searchString} in stack: \${stack}\`)`,
tag: (
strings: TemplateStringsArray,
...values: string[]
Expand All @@ -108,11 +108,21 @@ const files = {
assert(${cjsContextCheck}, 'Should have CJS context');
${preserveName}
${sourcemap.test('cjs')}
// Assert __esModule is unwrapped
import ('../ts/index.ts').then((m) => assert(
!(typeof m.default === 'object' && ('default' in m.default)),
));
exports.named = 'named';
`,

'mjs/index.mjs': `
import assert from 'assert';
export const mjsHasCjsContext = ${cjsContextCheck};
import ('pkg-commonjs').then((m) => assert(
!(typeof m.default === 'object' && ('default' in m.default)),
));
`,

'ts/index.ts': sourcemap.tag`
Expand Down Expand Up @@ -255,9 +265,9 @@ const files = {
[() => ${jsxCheck}, 'React is not defined'],
// These should throw unless allowJs is set
// [() => import('prefix/file'), "Cannot find package 'prefix'"],
// [() => import('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
// [() => import('file'), "Cannot find package 'file'"],
// [() => import ('prefix/file'), "Cannot find package 'prefix'"],
// [() => import ('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
// [() => import ('file'), "Cannot find package 'file'"],
);
`,

Expand All @@ -271,9 +281,9 @@ const files = {
'index.mjs': `
import { expectErrors } from '../../../expect-errors';
expectErrors(
[() => import('prefix/file'), "Cannot find package 'prefix'"],
[() => import('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
[() => import('file'), "Cannot find package 'file'"],
[() => import ('prefix/file'), "Cannot find package 'prefix'"],
[() => import ('paths-exact-match'), "Cannot find package 'paths-exact-match'"],
[() => import ('file'), "Cannot find package 'file'"],
);
`,
'index.cjs': `
Expand Down Expand Up @@ -366,7 +376,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
import './js/';
// No double .default.default in Dynamic Import
import('./js/index.js').then(m => {
import/* comment */('./js/index.js').then(m => {
if (typeof m.default === 'object') {
assert(
!('default' in m.default),
Expand All @@ -375,7 +385,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
}
});
const importWorksInEval = async () => await import('./js/index.js');
const importWorksInEval = async () => await import ('./js/index.js');
(0, eval)(importWorksInEval.toString())();
// .json
Expand All @@ -386,8 +396,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
// .cjs
import * as cjs from './cjs/index.cjs';
expectErrors(
[() => import('./cjs/index'), 'Cannot find module'],
[() => import('./cjs/'), 'Cannot find module'],
[() => import ('./cjs/index'), 'Cannot find module'],
[() => import ('./cjs/'), 'Cannot find module'],
${
isCommonJs
? `
Expand All @@ -401,8 +411,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
// .mjs
import * as mjs from './mjs/index.mjs';
expectErrors(
[() => import('./mjs/index'), 'Cannot find module'],
[() => import('./mjs/'), 'Cannot find module'],
[() => import ('./mjs/index'), 'Cannot find module'],
[() => import ('./mjs/'), 'Cannot find module'],
${
isCommonJs
? `
Expand All @@ -418,8 +428,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
// Unsupported files
expectErrors(
[() => import('./file.txt'), 'Unknown file extension'],
[() => import(${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
[() => import ('./file.txt'), 'Unknown file extension'],
[() => import (${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
${
isCommonJs
? `
Expand All @@ -433,7 +443,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
? '[() => require(\'./broken-syntax\'), \'Transform failed\'],'
: ''
}
[() => import('./broken-syntax'), 'Transform failed'],
[() => import ('./broken-syntax'), 'Transform failed'],
);
console.log(JSON.stringify({
Expand Down Expand Up @@ -510,7 +520,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
)};
// No double .default.default in Dynamic Import
import('./js/index.js').then(m => {
import/* comment */('./js/index.js').then(m => {
if (typeof m.default === 'object') {
assert(
!('default' in m.default),
Expand All @@ -527,8 +537,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
// .cjs
import * as cjs from './cjs/index.cjs';
expectErrors(
[() => import('./cjs/index'), 'Cannot find module'],
[() => import('./cjs/'), 'Cannot find module'],
[() => import ('./cjs/index'), 'Cannot find module'],
[() => import ('./cjs/'), 'Cannot find module'],
${
isCommonJs
? `
Expand All @@ -542,8 +552,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
// .mjs
import * as mjs from './mjs/index.mjs';
expectErrors(
[() => import('./mjs/index'), 'Cannot find module'],
[() => import('./mjs/'), 'Cannot find module'],
[() => import ('./mjs/index'), 'Cannot find module'],
[() => import ('./mjs/'), 'Cannot find module'],
${
isCommonJs
? `
Expand Down Expand Up @@ -578,9 +588,9 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
import './cts/index.cjs';
expectErrors(
// TODO:
// [() => import('./cts/index.cts'), 'Cannot find module'],
[() => import('./cts/index'), 'Cannot find module'],
[() => import('./cts/'), 'Cannot find module'],
// [() => import ('./cts/index.cts'), 'Cannot find module'],
[() => import ('./cts/index'), 'Cannot find module'],
[() => import ('./cts/'), 'Cannot find module'],
${
isCommonJs
? `
Expand All @@ -596,9 +606,9 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
import './mts/index.mjs';
expectErrors(
// TODO:
// [() => import('./mts/index.mts'), 'Cannot find module'],
[() => import('./mts/index'), 'Cannot find module'],
[() => import('./mts/'), 'Cannot find module'],
// [() => import ('./mts/index.mts'), 'Cannot find module'],
[() => import ('./mts/index'), 'Cannot find module'],
[() => import ('./mts/'), 'Cannot find module'],
${
isCommonJs
? `
Expand All @@ -612,8 +622,8 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
// Unsupported files
expectErrors(
[() => import('./file.txt'), 'Unknown file extension'],
[() => import(${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
[() => import ('./file.txt'), 'Unknown file extension'],
[() => import (${JSON.stringify(wasmPathUrl)}), 'Unknown file extension'],
${
isCommonJs
? `
Expand All @@ -627,7 +637,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
? '[() => require(\'./broken-syntax\'), \'Transform failed\'],'
: ''
}
[() => import('./broken-syntax'), 'Transform failed'],
[() => import ('./broken-syntax'), 'Transform failed'],
);
console.log(JSON.stringify({
Expand Down

0 comments on commit 5cdd50b

Please sign in to comment.