Skip to content

Commit f9b07ca

Browse files
igz0chargome
andauthored
fix(nextjs): preserve directive prologues in turbopack loaders (#20103)
Fixes #20101. ## Summary When `_experimental.turbopackApplicationKey` is enabled, the Next.js Turbopack loaders inject code for metadata/value propagation. The previous implementation relied on a regex that only handled a single directive prologue entry. For modules that start with multiple directives, such as: ```js "use strict"; "use client"; ``` the injected code could end up between the directives and break the `"use client"` classification. This replaces the regex-based insertion point detection with a small linear scanner that: - skips leading whitespace and comments - walks consecutive directive prologue entries - returns the insertion point immediately after the last directive ## Tests - added coverage for multiple directives - added coverage for comments between directives - added coverage for semicolon-free directives - added coverage for non-directive string literals that must not be skipped ## Verification Using the reproduction from #20101 with the patched `@sentry/nextjs` tarball: - `npm run build` completes successfully - `/_not-found` prerendering succeeds - the previous `TypeError: (0, g.useEffect) is not a function` no longer occurs --------- Co-authored-by: Charly Gomez <charly.gomez1310@gmail.com> Co-authored-by: Charly Gomez <charly.gomez@sentry.io>
1 parent 6049804 commit f9b07ca

File tree

4 files changed

+285
-17
lines changed

4 files changed

+285
-17
lines changed

packages/nextjs/src/config/loaders/moduleMetadataInjectionLoader.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { LoaderThis } from './types';
2-
import { SKIP_COMMENT_AND_DIRECTIVE_REGEX } from './valueInjectionLoader';
2+
import { findInjectionIndexAfterDirectives } from './valueInjectionLoader';
33

44
export type ModuleMetadataInjectionLoaderOptions = {
55
applicationKey: string;
@@ -39,7 +39,6 @@ export default function moduleMetadataInjectionLoader(
3939
`e._sentryModuleMetadata[(new e.Error).stack]=Object.assign({},e._sentryModuleMetadata[(new e.Error).stack],${metadata});` +
4040
'}catch(e){}}();';
4141

42-
return userCode.replace(SKIP_COMMENT_AND_DIRECTIVE_REGEX, match => {
43-
return match + injectedCode;
44-
});
42+
const injectionIndex = findInjectionIndexAfterDirectives(userCode);
43+
return `${userCode.slice(0, injectionIndex)}${injectedCode}${userCode.slice(injectionIndex)}`;
4544
}

packages/nextjs/src/config/loaders/valueInjectionLoader.ts

Lines changed: 143 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,150 @@
1-
// Rollup doesn't like if we put the directive regex as a literal (?). No idea why.
2-
/* oxlint-disable sdk/no-regexp-constructor */
3-
41
import type { LoaderThis } from './types';
52

63
export type ValueInjectionLoaderOptions = {
74
values: Record<string, unknown>;
85
};
96

10-
// We need to be careful not to inject anything before any `"use strict";`s or "use client"s or really any other directive.
11-
// As an additional complication directives may come after any number of comments.
12-
// This regex is shamelessly stolen from: https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/7f984482c73e4284e8b12a08dfedf23b5a82f0af/packages/bundler-plugin-core/src/index.ts#L535-L539
13-
export const SKIP_COMMENT_AND_DIRECTIVE_REGEX =
14-
// Note: CodeQL complains that this regex potentially has n^2 runtime. This likely won't affect realistic files.
15-
new RegExp('^(?:\\s*|/\\*(?:.|\\r|\\n)*?\\*/|//.*[\\n\\r])*(?:"[^"]*";?|\'[^\']*\';?)?');
7+
// We need to be careful not to inject anything before any `"use strict";`s or "use client"s or really any other directives.
8+
export function findInjectionIndexAfterDirectives(userCode: string): number {
9+
let index = 0;
10+
let lastDirectiveEndIndex: number | undefined;
11+
12+
while (index < userCode.length) {
13+
const statementStartIndex = skipWhitespaceAndComments(userCode, index);
14+
if (statementStartIndex === undefined) {
15+
return lastDirectiveEndIndex ?? 0;
16+
}
17+
18+
index = statementStartIndex;
19+
if (statementStartIndex === userCode.length) {
20+
return lastDirectiveEndIndex ?? statementStartIndex;
21+
}
22+
23+
const quote = userCode[statementStartIndex];
24+
if (quote !== '"' && quote !== "'") {
25+
return lastDirectiveEndIndex ?? statementStartIndex;
26+
}
27+
28+
const stringEndIndex = findStringLiteralEnd(userCode, statementStartIndex);
29+
if (stringEndIndex === undefined) {
30+
return lastDirectiveEndIndex ?? statementStartIndex;
31+
}
32+
33+
const statementEndIndex = findDirectiveTerminator(userCode, stringEndIndex);
34+
if (statementEndIndex === undefined) {
35+
return lastDirectiveEndIndex ?? statementStartIndex;
36+
}
37+
38+
index = statementEndIndex;
39+
lastDirectiveEndIndex = statementEndIndex;
40+
}
41+
42+
return lastDirectiveEndIndex ?? index;
43+
}
44+
45+
function skipWhitespaceAndComments(userCode: string, startIndex: number): number | undefined {
46+
let index = startIndex;
47+
48+
while (index < userCode.length) {
49+
const char = userCode[index];
50+
51+
if (char && /\s/.test(char)) {
52+
index += 1;
53+
continue;
54+
}
55+
56+
if (userCode.startsWith('//', index)) {
57+
const newlineIndex = userCode.indexOf('\n', index + 2);
58+
index = newlineIndex === -1 ? userCode.length : newlineIndex + 1;
59+
continue;
60+
}
61+
62+
if (userCode.startsWith('/*', index)) {
63+
const commentEndIndex = userCode.indexOf('*/', index + 2);
64+
if (commentEndIndex === -1) {
65+
return undefined;
66+
}
67+
68+
index = commentEndIndex + 2;
69+
continue;
70+
}
71+
72+
break;
73+
}
74+
75+
return index;
76+
}
77+
78+
function findStringLiteralEnd(userCode: string, startIndex: number): number | undefined {
79+
const quote = userCode[startIndex];
80+
let index = startIndex + 1;
81+
82+
while (index < userCode.length) {
83+
const char = userCode[index];
84+
85+
if (char === '\\') {
86+
index += 2;
87+
continue;
88+
}
89+
90+
if (char === quote) {
91+
return index + 1;
92+
}
93+
94+
if (char === '\n' || char === '\r') {
95+
return undefined;
96+
}
97+
98+
index += 1;
99+
}
100+
101+
return undefined;
102+
}
103+
104+
function findDirectiveTerminator(userCode: string, startIndex: number): number | undefined {
105+
let index = startIndex;
106+
107+
// Only a bare string literal followed by a statement terminator counts as a directive.
108+
while (index < userCode.length) {
109+
const char = userCode[index];
110+
111+
if (char === ';') {
112+
return index + 1;
113+
}
114+
115+
if (char === '\n' || char === '\r' || char === '}') {
116+
return index;
117+
}
118+
119+
if (char && /\s/.test(char)) {
120+
index += 1;
121+
continue;
122+
}
123+
124+
if (userCode.startsWith('//', index)) {
125+
return index;
126+
}
127+
128+
if (userCode.startsWith('/*', index)) {
129+
const commentEndIndex = userCode.indexOf('*/', index + 2);
130+
if (commentEndIndex === -1) {
131+
return undefined;
132+
}
133+
134+
const comment = userCode.slice(index + 2, commentEndIndex);
135+
if (comment.includes('\n') || comment.includes('\r')) {
136+
return index;
137+
}
138+
139+
index = commentEndIndex + 2;
140+
continue;
141+
}
142+
143+
return undefined;
144+
}
145+
146+
return index;
147+
}
16148

17149
/**
18150
* Set values on the global/window object at the start of a module.
@@ -36,7 +168,6 @@ export default function valueInjectionLoader(this: LoaderThis<ValueInjectionLoad
36168
.map(([key, value]) => `globalThis["${key}"] = ${JSON.stringify(value)};`)
37169
.join('');
38170

39-
return userCode.replace(SKIP_COMMENT_AND_DIRECTIVE_REGEX, match => {
40-
return match + injectedCode;
41-
});
171+
const injectionIndex = findInjectionIndexAfterDirectives(userCode);
172+
return `${userCode.slice(0, injectionIndex)}${injectedCode}${userCode.slice(injectionIndex)}`;
42173
}

packages/nextjs/test/config/moduleMetadataInjectionLoader.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,4 +131,30 @@ describe('moduleMetadataInjectionLoader', () => {
131131

132132
expect(result).toContain('"_sentryBundlerPluginAppKey:test-key-123":true');
133133
});
134+
135+
it('should inject after multiple directives', () => {
136+
const loaderThis = createLoaderThis('my-app');
137+
const userCode = '"use strict";\n"use client";\nimport React from \'react\';';
138+
139+
const result = moduleMetadataInjectionLoader.call(loaderThis, userCode);
140+
141+
const metadataIndex = result.indexOf('_sentryModuleMetadata');
142+
const clientDirectiveIndex = result.indexOf('"use client"');
143+
const importIndex = result.indexOf("import React from 'react';");
144+
145+
expect(metadataIndex).toBeGreaterThan(clientDirectiveIndex);
146+
expect(metadataIndex).toBeLessThan(importIndex);
147+
});
148+
149+
it('should inject after comments between multiple directives', () => {
150+
const loaderThis = createLoaderThis('my-app');
151+
const userCode = '"use strict";\n/* keep */\n"use client";\nimport React from \'react\';';
152+
153+
const result = moduleMetadataInjectionLoader.call(loaderThis, userCode);
154+
155+
const metadataIndex = result.indexOf('_sentryModuleMetadata');
156+
const clientDirectiveIndex = result.indexOf('"use client"');
157+
158+
expect(metadataIndex).toBeGreaterThan(clientDirectiveIndex);
159+
});
134160
});

packages/nextjs/test/config/valueInjectionLoader.test.ts

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, expect, it } from 'vitest';
22
import type { LoaderThis } from '../../src/config/loaders/types';
33
import type { ValueInjectionLoaderOptions } from '../../src/config/loaders/valueInjectionLoader';
4-
import valueInjectionLoader from '../../src/config/loaders/valueInjectionLoader';
4+
import valueInjectionLoader, { findInjectionIndexAfterDirectives } from '../../src/config/loaders/valueInjectionLoader';
55

66
const defaultLoaderThis = {
77
addDependency: () => undefined,
@@ -66,6 +66,23 @@ describe.each([[clientConfigLoaderThis], [instrumentationLoaderThis]])('valueInj
6666
expect(result).toMatch(';globalThis["foo"] = "bar";');
6767
});
6868

69+
it('should correctly insert values with a single-quoted directive', () => {
70+
const userCode = `
71+
'use client';
72+
import * as Sentry from '@sentry/nextjs';
73+
Sentry.init();
74+
`;
75+
76+
const result = valueInjectionLoader.call(loaderThis, userCode);
77+
78+
const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";');
79+
const clientDirectiveIndex = result.indexOf("'use client'");
80+
const importIndex = result.indexOf("import * as Sentry from '@sentry/nextjs';");
81+
82+
expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex);
83+
expect(injectionIndex).toBeLessThan(importIndex);
84+
});
85+
6986
it('should correctly insert values with directive and inline comments', () => {
7087
const userCode = `
7188
// test
@@ -149,4 +166,99 @@ describe.each([[clientConfigLoaderThis], [instrumentationLoaderThis]])('valueInj
149166
expect(result).toMatchSnapshot();
150167
expect(result).toMatch(';globalThis["foo"] = "bar";');
151168
});
169+
170+
it('should correctly insert values after multiple directives', () => {
171+
const userCode = `
172+
"use strict";
173+
"use client";
174+
import * as Sentry from '@sentry/nextjs';
175+
Sentry.init();
176+
`;
177+
178+
const result = valueInjectionLoader.call(loaderThis, userCode);
179+
180+
const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";');
181+
const clientDirectiveIndex = result.indexOf('"use client"');
182+
const importIndex = result.indexOf("import * as Sentry from '@sentry/nextjs';");
183+
184+
expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex);
185+
expect(injectionIndex).toBeLessThan(importIndex);
186+
});
187+
188+
it('should correctly insert values after comments between multiple directives', () => {
189+
const userCode = `
190+
"use strict";
191+
/* keep */
192+
"use client";
193+
import * as Sentry from '@sentry/nextjs';
194+
Sentry.init();
195+
`;
196+
197+
const result = valueInjectionLoader.call(loaderThis, userCode);
198+
199+
const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";');
200+
const clientDirectiveIndex = result.indexOf('"use client"');
201+
202+
expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex);
203+
});
204+
205+
it('should correctly insert values after semicolon-free directives', () => {
206+
const userCode = `
207+
"use strict"
208+
"use client"
209+
import * as Sentry from '@sentry/nextjs';
210+
Sentry.init();
211+
`;
212+
213+
const result = valueInjectionLoader.call(loaderThis, userCode);
214+
215+
const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";');
216+
const clientDirectiveIndex = result.indexOf('"use client"');
217+
218+
expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex);
219+
});
220+
});
221+
222+
describe('findInjectionIndexAfterDirectives', () => {
223+
it('returns the position immediately after the last directive', () => {
224+
const userCode = '"use strict";\n"use client";\nimport React from \'react\';';
225+
226+
expect(userCode.slice(findInjectionIndexAfterDirectives(userCode))).toBe("\nimport React from 'react';");
227+
});
228+
229+
it('returns the end of the input when the last directive reaches EOF', () => {
230+
const userCode = '"use strict";\n"use client";';
231+
232+
expect(findInjectionIndexAfterDirectives(userCode)).toBe(userCode.length);
233+
});
234+
235+
it('does not skip a string literal that is not a directive', () => {
236+
const userCode = '"use client" + suffix;';
237+
238+
expect(findInjectionIndexAfterDirectives(userCode)).toBe(0);
239+
});
240+
241+
it('does not treat an escaped quote at EOF as a closed directive', () => {
242+
const userCode = '"use client\\"';
243+
244+
expect(findInjectionIndexAfterDirectives(userCode)).toBe(0);
245+
});
246+
247+
it('returns 0 for an unterminated leading block comment', () => {
248+
const userCode = '/* unterminated';
249+
250+
expect(findInjectionIndexAfterDirectives(userCode)).toBe(0);
251+
});
252+
253+
it('returns the last complete directive when followed by an unterminated block comment', () => {
254+
const userCode = '"use client"; /* unterminated';
255+
256+
expect(findInjectionIndexAfterDirectives(userCode)).toBe('"use client";'.length);
257+
});
258+
259+
it('treats a block comment without a line break as part of the same statement', () => {
260+
const userCode = '"use client" /* comment */ + suffix;';
261+
262+
expect(findInjectionIndexAfterDirectives(userCode)).toBe(0);
263+
});
152264
});

0 commit comments

Comments
 (0)