-
-
Notifications
You must be signed in to change notification settings - Fork 82
fix(nextjs): Skip dynamic export in App Router routes when cacheComponents enabled #1245
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import * as path from 'path'; | |
| import { major, minVersion } from 'semver'; | ||
|
|
||
| // @ts-expect-error - magicast is ESM and TS complains about that. It works though | ||
| import { builders } from 'magicast'; | ||
| import { builders, parseModule } from 'magicast'; | ||
|
|
||
| export function getNextJsVersionBucket(version: string | undefined) { | ||
| if (!version) { | ||
|
|
@@ -25,6 +25,70 @@ export function getNextJsVersionBucket(version: string | undefined) { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Detects whether cacheComponents is enabled in the Next.js config. | ||
| * Returns true if cacheComponents is set to true, false otherwise. | ||
| */ | ||
| export function hasCacheComponentsEnabled(): boolean { | ||
| const nextConfigFiles = [ | ||
| 'next.config.js', | ||
| 'next.config.mjs', | ||
| 'next.config.ts', | ||
| 'next.config.mts', | ||
| 'next.config.cjs', | ||
| 'next.config.cts', | ||
| ]; | ||
|
|
||
| for (const configFile of nextConfigFiles) { | ||
| const configPath = path.join(process.cwd(), configFile); | ||
| if (!fs.existsSync(configPath)) { | ||
| continue; | ||
| } | ||
|
|
||
| try { | ||
| const configContent = fs.readFileSync(configPath, 'utf8'); | ||
|
|
||
| // First try a simple string check for common patterns | ||
| // This catches: cacheComponents: true, experimental: { cacheComponents: true } | ||
| if ( | ||
| /cacheComponents\s*:\s*true/.test(configContent) || | ||
| /experimental\s*:\s*\{\s*cacheComponents\s*:\s*true/.test(configContent) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Second regex condition is entirely redundantLow Severity The second regex |
||
| ) { | ||
| return true; | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regex matches cacheComponents in comments causing false positivesLow Severity The regex |
||
|
|
||
| // Try parsing with magicast for more complex cases | ||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| const mod = parseModule(configContent); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access | ||
| const nextConfig = mod.exports?.default?.$type | ||
| ? // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| mod.exports.default | ||
| : // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| mod.exports; | ||
|
|
||
| // Check for cacheComponents at root level or in experimental | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| if (nextConfig?.cacheComponents === true) { | ||
| return true; | ||
| } | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access | ||
| if (nextConfig?.experimental?.cacheComponents === true) { | ||
| return true; | ||
| } | ||
| } catch { | ||
| // If magicast parsing fails, we already checked with regex above | ||
| } | ||
| } catch { | ||
| // If we can't read the file, continue to the next one | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| export function getMaybeAppDirLocation() { | ||
| const maybeAppDirPath = path.join(process.cwd(), 'app'); | ||
| const maybeSrcAppDirPath = path.join(process.cwd(), 'src', 'app'); | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing blank line between import and class declaration
Low Severity
When
includeDynamicisfalseandlogsEnabledistrue, thesentryImportstring ends with a single newline anddynamicExportis empty, so the generated file has no blank line between theimportstatement and theclass SentryExampleAPIErrordeclaration. Previously, thedynamicExportcontent (which ended with\n\n) always provided that visual and linter-expected separation. This could trigger lint rules likeimport/newline-after-importin the generated example file.