Skip to content

fix(nextjs): Pull transpileClientSDK option from correct location #5516

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.

Sign up for GitHub

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

Merged
lobsterkatie merged 4 commits into from
Aug 4, 2022
Merged

fix(nextjs): Pull transpileClientSDK option from correct location #5516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c811a5a
create separate types for before and after deleting `sentry` property
lobsterkatie Aug 3, 2022
047a38e
pull `transpileClientSDK` option from correct location
lobsterkatie Aug 3, 2022
47c8dac
fix tests
lobsterkatie Aug 3, 2022
4481089
add test for removing sentry
lobsterkatie Aug 3, 2022
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
10 changes: 7 additions & 3 deletions packages/nextjs/src/config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ export function withSentryConfig(
// `defaults` in order to pass them along to the user's function
if (typeof exportedUserNextConfig === 'function') {
return function (phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
const userNextConfigObject = exportedUserNextConfig(phase, defaults);
let userNextConfigObject = exportedUserNextConfig(phase, defaults);

// Next 12.2.3+ warns about non-canonical properties on `userNextConfig`, so grab and then remove the `sentry`
// property there. Where we actually need it is in the webpack config function we're going to create, so pass it
// to `constructWebpackConfigFunction` so that it will be in the created function's closure.
const { sentry: userSentryOptions } = userNextConfigObject;
delete userNextConfigObject.sentry;
// Remind TS that there's now no `sentry` property
userNextConfigObject = userNextConfigObject as NextConfigObject;
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad Aug 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we just do ...(userNextConfigObject as NextConfigObject)? I'd rather not change from const -> let

we can also just declare a new var - modifiedUserNextConfigObject?

Copy link
Copy Markdown
Member Author

@lobsterkatie lobsterkatie Aug 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not change from const -> let

Why?

(FYI: I'm going to merge this regardless, because I'd like to get this bugfix out, but we can always revisit it later.)


return {
...userNextConfigObject,
Expand All @@ -41,9 +43,11 @@ export function withSentryConfig(
// for a more thorough explanation of what we're doing here.)
const { sentry: userSentryOptions } = exportedUserNextConfig;
delete exportedUserNextConfig.sentry;
// Remind TS that there's now no `sentry` property
const userNextConfigObject = exportedUserNextConfig as NextConfigObject;

return {
...exportedUserNextConfig,
webpack: constructWebpackConfigFunction(exportedUserNextConfig, userSentryWebpackPluginOptions, userSentryOptions),
...userNextConfigObject,
webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions),
};
}
14 changes: 12 additions & 2 deletions packages/nextjs/src/config/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,18 @@ export type SentryWebpackPlugin = WebpackPluginInstance & { options: SentryWebpa
* Overall Nextjs config
*/

export type ExportedNextConfig = NextConfigObject | NextConfigFunction;
// The first argument to `withSentryConfig` (which is the user's next config) may contain a `sentry` key, which we'll
// remove once we've captured it, in order to prevent nextjs from throwing warnings. Since it's only in there
// temporarily, we don't include it in the main `NextConfigObject` or `NextConfigFunction` types.
export type ExportedNextConfig = NextConfigObjectWithSentry | NextConfigFunctionWithSentry;

export type NextConfigObjectWithSentry = NextConfigObject & {
sentry?: UserSentryOptions;
};
export type NextConfigFunctionWithSentry = (
phase: string,
defaults: { defaultConfig: NextConfigObject },
) => NextConfigObjectWithSentry;

export type NextConfigObject = {
// custom webpack options
Expand All @@ -21,7 +32,6 @@ export type NextConfigObject = {
basePath?: string;
// config which will be available at runtime
publicRuntimeConfig?: { [key: string]: unknown };
sentry?: UserSentryOptions;
};

export type UserSentryOptions = {
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/src/config/webpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export function constructWebpackConfigFunction(
// who want to support such browsers, `transpileClientSDK` allows them to force the SDK code to go through the same
// transpilation that their code goes through. We don't turn this on by default because it increases bundle size
// fairly massively.
if (!isServer && userNextConfig.sentry?.transpileClientSDK) {
if (!isServer && userSentryOptions?.transpileClientSDK) {
// Find all loaders which apply transpilation to user code
const transpilationRules = findTranspilationRules(newConfig.module?.rules, projectDir);

Expand Down
Loading