Skip to content

Commit a74d6e6

Browse files
committed
switch to AST-based wrapping
1 parent 57007c9 commit a74d6e6

File tree

9 files changed

+333
-206
lines changed

9 files changed

+333
-206
lines changed

packages/sveltekit/.eslintrc.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,4 @@ module.exports = {
1919
},
2020
],
2121
extends: ['../../.eslintrc.js'],
22-
ignorePatterns: ['scripts/**/*', 'src/vite/templates/**/*'],
2322
};

packages/sveltekit/package.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,24 +28,24 @@
2828
"@sentry/types": "7.49.0",
2929
"@sentry/utils": "7.49.0",
3030
"@sentry/vite-plugin": "^0.6.0",
31-
"magic-string": "^0.30.0",
32-
"rollup": "^3.20.2",
31+
"magicast": "0.2.4",
3332
"sorcery": "0.11.0"
3433
},
3534
"devDependencies": {
3635
"@sveltejs/kit": "^1.11.0",
36+
"rollup": "^3.20.2",
3737
"svelte": "^3.44.0",
3838
"typescript": "^4.9.3",
3939
"vite": "4.0.0"
4040
},
4141
"scripts": {
4242
"build": "run-p build:transpile build:types",
4343
"build:dev": "yarn build",
44-
"build:transpile": "ts-node scripts/buildRollup.ts",
44+
"build:transpile": "rollup -c rollup.npm.config.js --bundleConfigAsCjs",
4545
"build:types": "tsc -p tsconfig.types.json",
4646
"build:watch": "run-p build:transpile:watch build:types:watch",
4747
"build:dev:watch": "yarn build:watch",
48-
"build:transpile:watch": "nodemon --ext ts --watch src scripts/buildRollup.ts",
48+
"build:transpile:watch": "rollup -c rollup.npm.config.js --watch",
4949
"build:types:watch": "tsc -p tsconfig.types.json --watch",
5050
"build:tarball": "ts-node ../../scripts/prepack.ts && npm pack ./build",
5151
"circularDepCheck": "madge --circular src/index.client.ts && madge --circular src/index.server.ts && madge --circular src/index.types.ts",
Lines changed: 11 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,13 @@
11
import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js';
22

3-
export default [
4-
5-
...makeNPMConfigVariants(
6-
makeBaseNPMConfig({
7-
entrypoints: ['src/index.server.ts', 'src/index.client.ts', 'src/client/index.ts', 'src/server/index.ts'],
8-
packageSpecificConfig: {
9-
external: ['$app/stores'],
10-
output: {
11-
dynamicImportInCjs: true,
12-
}
13-
},
14-
}),
15-
),
16-
// Templates for load function wrappers
17-
...makeNPMConfigVariants(
18-
makeBaseNPMConfig({
19-
entrypoints: [
20-
'src/vite/templates/universalLoadTemplate.ts',
21-
'src/vite/templates/serverLoadTemplate.ts',
22-
],
23-
24-
packageSpecificConfig: {
25-
output: {
26-
// Preserve the original file structure (i.e., so that everything is still relative to `src`)
27-
entryFileNames: 'vite/templates/[name].js',
28-
29-
// this is going to be add-on code, so it doesn't need the trappings of a full module (and in fact actively
30-
// shouldn't have them, lest they muck with the module to which we're adding it)
31-
sourcemap: false,
32-
esModule: false,
33-
34-
// make it so Rollup calms down about the fact that we're combining default and named exports
35-
exports: 'named',
36-
},
37-
external: [
38-
'@sentry/sveltekit',
39-
'__SENTRY_WRAPPING_TARGET_FILE__',
40-
],
41-
},
42-
}),
43-
),
44-
];
3+
export default makeNPMConfigVariants(
4+
makeBaseNPMConfig({
5+
entrypoints: ['src/index.server.ts', 'src/index.client.ts', 'src/client/index.ts', 'src/server/index.ts'],
6+
packageSpecificConfig: {
7+
external: ['$app/stores'],
8+
output: {
9+
dynamicImportInCjs: true,
10+
}
11+
},
12+
}),
13+
);

packages/sveltekit/scripts/buildRollup.ts

Lines changed: 0 additions & 24 deletions
This file was deleted.
Lines changed: 118 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
/* eslint-disable @sentry-internal/sdk/no-optional-chaining */
2-
import * as fs from 'fs';
3-
import * as path from 'path';
4-
import type { SourceMap } from 'rollup';
5-
import { rollup } from 'rollup';
2+
import type {
3+
ExportNamedDeclaration,
4+
FunctionDeclaration,
5+
Program,
6+
VariableDeclaration,
7+
VariableDeclarator,
8+
} from '@babel/types';
9+
import type { ProxifiedModule } from 'magicast';
10+
import { builders, generateCode, parseModule } from 'magicast';
611
import type { Plugin } from 'vite';
712

8-
// Just a simple placeholder to make referencing module consistent
9-
const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module';
10-
11-
// Needs to end in .cjs in order for the `commonjs` plugin to pick it up
12-
const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET_FILE__.js';
13-
1413
export type AutoInstrumentSelection = {
1514
/**
1615
* If this flag is `true`, the Sentry plugins will automatically instrument the `load` function of
@@ -41,21 +40,6 @@ type AutoInstrumentPluginOptions = AutoInstrumentSelection & {
4140
* @returns the plugin
4241
*/
4342
export async function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptions): Promise<Plugin> {
44-
const universalLoadTemplatePath = path.resolve(__dirname, 'templates', 'universalLoadTemplate.js');
45-
const universalLoadTemplate = (await fs.promises.readFile(universalLoadTemplatePath, 'utf-8')).toString();
46-
47-
const serverLoadTemplatePath = path.resolve(__dirname, 'templates', 'serverLoadTemplate.js');
48-
const serverLoadTemplate = (await fs.promises.readFile(serverLoadTemplatePath, 'utf-8')).toString();
49-
50-
const universalLoadWrappingCode = universalLoadTemplate.replace(
51-
/__SENTRY_WRAPPING_TARGET_FILE__/g,
52-
WRAPPING_TARGET_MODULE_NAME,
53-
);
54-
const serverLoadWrappingCode = serverLoadTemplate.replace(
55-
/__SENTRY_WRAPPING_TARGET_FILE__/g,
56-
WRAPPING_TARGET_MODULE_NAME,
57-
);
58-
5943
const { load: shouldWrapLoad, serverLoad: shouldWrapServerLoad, debug } = options;
6044

6145
return {
@@ -71,7 +55,8 @@ export async function makeAutoInstrumentationPlugin(options: AutoInstrumentPlugi
7155
if (shouldApplyUniversalLoadWrapper) {
7256
// eslint-disable-next-line no-console
7357
debug && console.log('[Sentry] Applying universal load wrapper to', id);
74-
return await wrapUserCode(universalLoadWrappingCode, userCode);
58+
const wrappedCode = wrapLoad(userCode, 'wrapLoadWithSentry');
59+
return { code: wrappedCode, map: null };
7560
}
7661

7762
const shouldApplyServerLoadWrapper =
@@ -82,7 +67,8 @@ export async function makeAutoInstrumentationPlugin(options: AutoInstrumentPlugi
8267
if (shouldApplyServerLoadWrapper) {
8368
// eslint-disable-next-line no-console
8469
debug && console.log('[Sentry] Applying server load wrapper to', id);
85-
return await wrapUserCode(serverLoadWrappingCode, userCode);
70+
const wrappedCode = wrapLoad(userCode, 'wrapServerLoadWithSentry');
71+
return { code: wrappedCode, map: null };
8672
}
8773

8874
return null;
@@ -91,62 +77,118 @@ export async function makeAutoInstrumentationPlugin(options: AutoInstrumentPlugi
9177
}
9278

9379
/**
94-
* Uses rollup to bundle the wrapper code and the user code together, so that we can use rollup's source map support.
95-
* This works analogously to our NextJS wrapping solution.
96-
* The one exception is that we don't pass in any source map. This is because generating the userCode's
97-
* source map generally works but it breaks SvelteKit's source map generation for some reason.
98-
* Not passing a map actually works and things are still mapped correctly in the end.
99-
* No Sentry code is visible in the final source map.
100-
* @see {@link file:///./../../../nextjs/src/config/loaders/wrappingLoader.ts} for more details.
80+
* Applies the wrapLoadWithSentry wrapper to the user's load functions
10181
*/
102-
async function wrapUserCode(
103-
wrapperCode: string,
104-
userModuleCode: string,
105-
): Promise<{ code: string; map?: SourceMap | null }> {
106-
const rollupBuild = await rollup({
107-
input: SENTRY_WRAPPER_MODULE_NAME,
108-
109-
plugins: [
110-
{
111-
name: 'virtualize-sentry-wrapper-modules',
112-
resolveId: id => {
113-
if (id === SENTRY_WRAPPER_MODULE_NAME || id === WRAPPING_TARGET_MODULE_NAME) {
114-
return id;
115-
} else {
116-
return null;
117-
}
118-
},
119-
load(id) {
120-
if (id === SENTRY_WRAPPER_MODULE_NAME) {
121-
return wrapperCode;
122-
} else if (id === WRAPPING_TARGET_MODULE_NAME) {
123-
return {
124-
code: userModuleCode,
125-
// map: userModuleSourceMap,
126-
};
127-
} else {
128-
return null;
129-
}
130-
},
131-
},
132-
],
82+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
83+
function wrapLoad(
84+
userCode: Readonly<string>,
85+
wrapperFunction: 'wrapLoadWithSentry' | 'wrapServerLoadWithSentry',
86+
): string {
87+
const mod = parseModule(userCode);
88+
89+
const modAST = mod.exports.$ast as Program;
90+
const namedExports = modAST.body.filter(
91+
(node): node is ExportNamedDeclaration => node.type === 'ExportNamedDeclaration',
92+
);
13393

134-
external: sourceId => sourceId !== SENTRY_WRAPPER_MODULE_NAME && sourceId !== WRAPPING_TARGET_MODULE_NAME,
94+
let wrappedSucessfully = false;
95+
namedExports.forEach(modExport => {
96+
const declaration = modExport.declaration;
97+
if (!declaration) {
98+
return;
99+
}
100+
if (declaration.type === 'FunctionDeclaration') {
101+
if (!declaration.id || declaration.id.name !== 'load') {
102+
return;
103+
}
104+
const declarationCode = generateCode(declaration).code;
105+
mod.exports.load = builders.raw(`${wrapperFunction}(${declarationCode.replace('load', '_load')})`);
106+
// because of an issue with magicast, we need to remove the original export
107+
modAST.body = modAST.body.filter(node => node !== modExport);
108+
wrappedSucessfully = true;
109+
} else if (declaration.type === 'VariableDeclaration') {
110+
declaration.declarations.forEach(declarator => {
111+
wrappedSucessfully = wrapDeclarator(declarator, wrapperFunction);
112+
});
113+
}
114+
});
135115

136-
context: 'this',
116+
if (wrappedSucessfully) {
117+
return generateFinalCode(mod, wrapperFunction);
118+
}
137119

138-
makeAbsoluteExternalsRelative: false,
120+
// If we're here, we know that we didn't find a directly exported `load` function yet.
121+
// We need to look for it in the top level declarations in case it's declared and exported separately.
122+
// First case: top level variable declaration
123+
const topLevelVariableDeclarations = modAST.body.filter(
124+
(statement): statement is VariableDeclaration => statement.type === 'VariableDeclaration',
125+
);
139126

140-
onwarn: (_warning, _warn) => {
141-
// Suppress all warnings - we don't want to bother people with this output
142-
// _warn(_warning); // uncomment to debug
143-
},
127+
topLevelVariableDeclarations.forEach(declaration => {
128+
declaration.declarations.forEach(declarator => {
129+
wrappedSucessfully = wrapDeclarator(declarator, wrapperFunction);
130+
});
144131
});
145132

146-
const finalBundle = await rollupBuild.generate({
147-
format: 'esm',
148-
sourcemap: 'hidden',
133+
if (wrappedSucessfully) {
134+
return generateFinalCode(mod, wrapperFunction);
135+
}
136+
137+
// Second case: top level function declaration
138+
// This is the most intrusive modification, as we need to replace a top level function declaration with a
139+
// variable declaration and a function assignment. This changes the spacing formatting of the declarations
140+
// but the line numbers should stay the same
141+
const topLevelFunctionDeclarations = modAST.body.filter(
142+
(statement): statement is FunctionDeclaration => statement.type === 'FunctionDeclaration',
143+
);
144+
145+
topLevelFunctionDeclarations.forEach(declaration => {
146+
if (!declaration.id || declaration.id.name !== 'load') {
147+
return;
148+
}
149+
150+
const stmtIndex = modAST.body.indexOf(declaration);
151+
const declarationCode = generateCode(declaration).code;
152+
const wrappedFunctionBody = builders.raw(`${wrapperFunction}(${declarationCode.replace('load', '_load')})`);
153+
const stringifiedFunctionBody = generateCode(wrappedFunctionBody, {}).code;
154+
155+
const tmpMod = parseModule(`const load = ${stringifiedFunctionBody}`);
156+
const newDeclarationNode = (tmpMod.$ast as Program).body[0];
157+
const nodeWithAdjustedLoc = {
158+
...newDeclarationNode,
159+
loc: {
160+
...declaration.loc,
161+
},
162+
};
163+
164+
// @ts-ignore - this works, magicast can handle this assignement although the types disagree
165+
modAST.body[stmtIndex] = nodeWithAdjustedLoc;
166+
wrappedSucessfully = true;
149167
});
150168

151-
return finalBundle.output[0];
169+
if (wrappedSucessfully) {
170+
return generateFinalCode(mod, wrapperFunction);
171+
}
172+
173+
// nothing found, so we just return the original code
174+
return userCode;
175+
}
176+
177+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
178+
function generateFinalCode(mod: ProxifiedModule<any>, wrapperFunction: string): string {
179+
const { code } = generateCode(mod);
180+
return `import { ${wrapperFunction} } from '@sentry/sveltekit'; ${code}`;
181+
}
182+
183+
function wrapDeclarator(declarator: VariableDeclarator, wrapperFunction: string): boolean {
184+
// @ts-ignore - id should always have a name in this case
185+
if (!declarator.id || declarator.id.name !== 'load') {
186+
return false;
187+
}
188+
const declarationInitCode = declarator.init;
189+
// @ts-ignore - we can just place a string here, magicast will convert it to a node
190+
const stringifiedCode = generateCode(declarationInitCode).code;
191+
// @ts-ignore - we can just place a string here, magicast will convert it to a node
192+
declarator.init = `${wrapperFunction}(${stringifiedCode})`;
193+
return true;
152194
}

packages/sveltekit/src/vite/templates/serverLoadTemplate.ts

Lines changed: 0 additions & 12 deletions
This file was deleted.

packages/sveltekit/src/vite/templates/universalLoadTemplate.ts

Lines changed: 0 additions & 12 deletions
This file was deleted.

0 commit comments

Comments
 (0)