Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: strip source map when unused #374

Merged
merged 3 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/cjs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
createFilesMatcher,
} from 'get-tsconfig';
import type { TransformOptions } from 'esbuild';
import { installSourceMapSupport } from '../source-map';
import { installSourceMapSupport, shouldStripSourceMap, stripSourceMap } from '../source-map';
import { transformSync, transformDynamicImport } from '../utils/transform';
import { resolveTsPath } from '../utils/resolve-ts-path';
import { isESM } from '../utils/esm-pattern';
Expand Down Expand Up @@ -67,6 +67,12 @@ const transformer = (
}

let code = fs.readFileSync(filePath, 'utf8');

// Strip source maps if originally disabled
if (shouldStripSourceMap) {
code = stripSourceMap(code);
}

if (filePath.endsWith('.cjs')) {
// Contains native ESM check
const transformed = transformDynamicImport(filePath, code);
Expand Down
11 changes: 9 additions & 2 deletions src/esm/loaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import type {
import type { TransformOptions } from 'esbuild';
import { transform, transformDynamicImport } from '../utils/transform';
import { resolveTsPath } from '../utils/resolve-ts-path';
import { installSourceMapSupport, shouldStripSourceMap, stripSourceMap } from '../source-map';
import {
applySourceMap,
tsconfigPathsMatcher,
fileMatcher,
tsExtensionsPattern,
Expand All @@ -19,6 +19,8 @@ import {
type NodeError,
} from './utils.js';

const applySourceMap = installSourceMapSupport();

const isDirectoryPattern = /\/(?:$|\?)/;

type NextResolve = (
Expand Down Expand Up @@ -274,7 +276,12 @@ export const load: LoadHook = async function (
}

const filePath = url.startsWith('file://') ? fileURLToPath(url) : url;
const code = loaded.source.toString();
let code = loaded.source.toString();

// Strip source maps if originally disabled
if (shouldStripSourceMap) {
code = stripSourceMap(code);
}

if (
// Support named imports in JSON modules
Expand Down
3 changes: 0 additions & 3 deletions src/esm/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@ import {
createPathsMatcher,
createFilesMatcher,
} from 'get-tsconfig';
import { installSourceMapSupport } from '../source-map';
import { getPackageType } from './package-json.js';

export const applySourceMap = installSourceMapSupport();

const tsconfig = (
process.env.TSX_TSCONFIG_PATH
? {
Expand Down
24 changes: 20 additions & 4 deletions src/source-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,31 @@ type PortMessage = {
map: RawSourceMap;
};

const inlineSourceMapPrefix = '\n//# sourceMappingURL=data:application/json;base64,';
// If Node.js has source map disabled, we should strip source maps to speed up processing
export const shouldStripSourceMap = (
('sourceMapsEnabled' in process)
&& process.sourceMapsEnabled === false
);

export function installSourceMapSupport(
const sourceMapPrefix = '\n//# sourceMappingURL=';

export const stripSourceMap = (code: string) => {
const sourceMapIndex = code.indexOf(sourceMapPrefix);
if (sourceMapIndex !== -1) {
return code.slice(0, sourceMapIndex);
}
return code;
};

const inlineSourceMapPrefix = `${sourceMapPrefix}data:application/json;base64,`;

export const installSourceMapSupport = (
/**
* To support Node v20 where loaders are executed in its own thread
* https://nodejs.org/docs/latest-v20.x/api/esm.html#globalpreload
*/
loaderPort?: MessagePort,
) {
) => {
const hasNativeSourceMapSupport = (
/**
* Check if native source maps are supported by seeing if the API is available
Expand Down Expand Up @@ -77,4 +93,4 @@ export function installSourceMapSupport(
}
return code;
};
}
};
63 changes: 32 additions & 31 deletions src/utils/debug.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,37 @@
export const time = <Argument>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export const time = <T extends (...args: any[]) => unknown>(
name: string,
_function: (...args: Argument[]) => unknown,
) => function (
this: unknown,
...args: Argument[]
) {
const timeStart = Date.now();
const logTimeElapsed = () => {
const elapsed = Date.now() - timeStart;
_function: T,
threshold = 100,
): T => function (
this: unknown,
...args: Parameters<T>
) {
const timeStart = Date.now();
const logTimeElapsed = () => {
const elapsed = Date.now() - timeStart;

if (elapsed > 10) {
// console.log({
// name,
// args,
// elapsed,
// });
}
};
if (elapsed > threshold) {
console.log(name, {
args,
elapsed,
});
}
};

const result = Reflect.apply(_function, this, args);
if (
result
const result = Reflect.apply(_function, this, args);
if (
result
&& typeof result === 'object'
&& 'then' in result
) {
(result as Promise<unknown>).then(
logTimeElapsed,
// Ignore error in this chain
() => {},
);
} else {
logTimeElapsed();
}
return result;
};
) {
(result as Promise<unknown>).then(
logTimeElapsed,
// Ignore error in this chain
() => {},
);
} else {
logTimeElapsed();
}
return result;
} as T;
11 changes: 8 additions & 3 deletions src/utils/esm-pattern.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { parseEsm } from './es-module-lexer';

/*
TODO: Add tests
Previously, this regex was used as a naive ESM catch,
but turns out regex is slower than the lexer so removing
it made the check faster.

Catches:
import a from 'b'
import 'b';
Expand All @@ -12,11 +16,12 @@ Doesn't catch:
EXPORT{a}
exports.a = 1
module.exports = 1
*/

const esmPattern = /\b(?:import|export)\b/;
*/

export const isESM = (code: string) => {
if (esmPattern.test(code)) {
if (code.includes('import') || code.includes('export')) {
const [imports, exports] = parseEsm(code);
return imports.length > 0 || exports.length > 0;
}
Expand Down