- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.7k
          feat(nextjs): Support distDir Next.js option
          #3990
        
          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
          
     Merged
      
      
    
  
     Merged
                    Changes from all commits
      Commits
    
    
            Show all changes
          
          
            18 commits
          
        
        Select commit
          Hold shift + click to select a range
      
      4d88272
              
                feat(nextjs): Support `distDir` Next.js option
              
              
                iker-barriocanal cb94618
              
                Move the comment back to its original place
              
              
                iker-barriocanal 042223b
              
                Separate `includeSources` to another file
              
              
                iker-barriocanal 8014413
              
                Add unit tests to `includeDistDir`
              
              
                iker-barriocanal 37e15ee
              
                Rename mocks
              
              
                iker-barriocanal 9295eaa
              
                fix types
              
              
                iker-barriocanal 1a3aed0
              
                Use `??` instead of `||`
              
              
                iker-barriocanal 344fd39
              
                Add tests for `getWebpackPluginOptions`
              
              
                iker-barriocanal a940c07
              
                Remove redundant comments
              
              
                iker-barriocanal f5f1f25
              
                Separate tests by test name
              
              
                iker-barriocanal ef4cffd
              
                Generalize the use case
              
              
                iker-barriocanal 3120b7c
              
                Add note about overriding values
              
              
                iker-barriocanal 718b269
              
                Merge branch 'master' into iker/feat/nextjs-distdir
              
              
                iker-barriocanal de8ff3e
              
                Add test for duplicated props in includeNextjsProps
              
              
                iker-barriocanal ca0a409
              
                feedback
              
              
                iker-barriocanal 3f1df7c
              
                Use `Array.from` instead of spread array
              
              
                iker-barriocanal 3ee1215
              
                Fix type on test
              
              
                iker-barriocanal 3ed2d01
              
                Merge branch 'master' into iker/feat/nextjs-distdir
              
              
                iker-barriocanal File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
        
          
          
            125 changes: 125 additions & 0 deletions
          
          125 
        
  packages/nextjs/src/config/nextConfigToWebpackPluginConfig.ts
  
  
      
      
   
        
      
      
    
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              | Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| import { NextConfigObject, SentryWebpackPluginOptions } from './types'; | ||
|  | ||
| /** | ||
| * About types: | ||
| * It's not possible to set strong types because they end up forcing you to explicitly | ||
| * set `undefined` for properties you don't want to include, which is quite | ||
| * inconvenient. The workaround to this is to relax type requirements at some point, | ||
| * which means not enforcing types (why have strong typing then?) and still having code | ||
| * that is hard to read. | ||
| */ | ||
|  | ||
| /** | ||
| * Next.js properties that should modify the webpack plugin properties. | ||
| * They should have an includer function in the map. | ||
| */ | ||
| export const SUPPORTED_NEXTJS_PROPERTIES = ['distDir']; | ||
|  | ||
| export type PropIncluderFn = ( | ||
| nextConfig: NextConfigObject, | ||
| sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>, | ||
| ) => Partial<SentryWebpackPluginOptions>; | ||
|  | ||
| export const PROPS_INCLUDER_MAP: Record<string, PropIncluderFn> = { | ||
| distDir: includeDistDir, | ||
| }; | ||
|  | ||
| /** | ||
| * Creates a new Sentry Webpack Plugin config from the given one, including all available | ||
| * properties in the Nextjs Config. | ||
| * | ||
| * @param nextConfig User's Next.js config. | ||
| * @param sentryWebpackPluginOptions User's Sentry Webpack Plugin config. | ||
| * @returns New Sentry Webpack Plugin Config. | ||
| */ | ||
| export default function includeAllNextjsProps( | ||
| nextConfig: NextConfigObject, | ||
| sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>, | ||
| ): Partial<SentryWebpackPluginOptions> { | ||
| return includeNextjsProps(nextConfig, sentryWebpackPluginOptions, PROPS_INCLUDER_MAP, SUPPORTED_NEXTJS_PROPERTIES); | ||
| } | ||
|  | ||
| /** | ||
| * Creates a new Sentry Webpack Plugin config from the given one, and applying the corresponding | ||
| * modifications to the given next properties. If more than one option generates the same | ||
| * properties, the values generated last will override previous ones. | ||
| * | ||
| * @param nextConfig User's Next.js config. | ||
| * @param sentryWebpackPluginOptions User's Sentry Webapck Plugin config. | ||
| * @param nextProps Next.js config's properties that should modify webpack plugin properties. | ||
| * @returns New Sentry Webpack Plugin config. | ||
| */ | ||
| export function includeNextjsProps( | ||
| nextConfig: NextConfigObject, | ||
| sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>, | ||
| propsIncluderMap: Record<string, PropIncluderFn>, | ||
| nextProps: string[], | ||
| ): Partial<SentryWebpackPluginOptions> { | ||
| const propsToInclude = Array.from(new Set(nextProps)); | ||
| return ( | ||
| propsToInclude | ||
| // Types are not strict enought to ensure there's a function in the map | ||
| .filter(prop => propsIncluderMap[prop]) | ||
| .map(prop => propsIncluderMap[prop](nextConfig, sentryWebpackPluginOptions)) | ||
| .reduce((prev, current) => ({ ...prev, ...current }), {}) | ||
| ); | ||
|         
                  iker-barriocanal marked this conversation as resolved.
              Show resolved
            Hide resolved | ||
| } | ||
|  | ||
| /** | ||
| * Creates a new Sentry Webpack Plugin config with the `distDir` option from Next.js config | ||
| * in the `include` property, if `distDir` is provided. | ||
| * | ||
| * If no `distDir` is provided, the Webpack Plugin config doesn't change and the same object is returned. | ||
| * If no `include` has been defined defined, the `distDir` value is assigned. | ||
| * The `distDir` directory is merged to the directories in `include`, if defined. | ||
| * Duplicated paths are removed while merging. | ||
| * | ||
| * @param nextConfig User's Next.js config | ||
| * @param sentryWebpackPluginOptions User's Sentry Webpack Plugin config | ||
| * @returns Sentry Webpack Plugin config | ||
| */ | ||
| export function includeDistDir( | ||
| nextConfig: NextConfigObject, | ||
| sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>, | ||
| ): Partial<SentryWebpackPluginOptions> { | ||
| if (!nextConfig.distDir) { | ||
| return sentryWebpackPluginOptions; | ||
| } | ||
| // It's assumed `distDir` is a string as that's what Next.js is expecting. If it's not, Next.js itself will complain | ||
| const usersInclude = sentryWebpackPluginOptions.include; | ||
|  | ||
| let sourcesToInclude; | ||
| if (typeof usersInclude === 'undefined') { | ||
| sourcesToInclude = nextConfig.distDir; | ||
| } else if (typeof usersInclude === 'string') { | ||
| sourcesToInclude = usersInclude === nextConfig.distDir ? usersInclude : [usersInclude, nextConfig.distDir]; | ||
| } else if (Array.isArray(usersInclude)) { | ||
| sourcesToInclude = Array.from(new Set(usersInclude.concat(nextConfig.distDir as string))); | ||
| } else { | ||
| // Object | ||
| if (Array.isArray(usersInclude.paths)) { | ||
| const uniquePaths = Array.from(new Set(usersInclude.paths.concat(nextConfig.distDir as string))); | ||
| sourcesToInclude = { ...usersInclude, paths: uniquePaths }; | ||
| } else if (typeof usersInclude.paths === 'undefined') { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| 'Sentry Logger [Warn]:', | ||
| `An object was set in \`include\` but no \`paths\` was provided, so added the \`distDir\`: "${nextConfig.distDir}"\n` + | ||
| 'See https://github.com/getsentry/sentry-webpack-plugin#optionsinclude', | ||
| ); | ||
| sourcesToInclude = { ...usersInclude, paths: [nextConfig.distDir] }; | ||
| } else { | ||
| // eslint-disable-next-line no-console | ||
| console.error( | ||
| 'Sentry Logger [Error]:', | ||
| 'Found unexpected object in `include.paths`\n' + | ||
| 'See https://github.com/getsentry/sentry-webpack-plugin#optionsinclude', | ||
| ); | ||
| // Keep the same object even if it's incorrect, so that the user can get a more precise error from sentry-cli | ||
| // Casting to `any` for TS not complaining about it being `unknown` | ||
| sourcesToInclude = usersInclude as any; | ||
| } | ||
| } | ||
|  | ||
| return { ...sentryWebpackPluginOptions, include: sourcesToInclude }; | ||
| } | ||
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
        
          
          
            117 changes: 117 additions & 0 deletions
          
          117 
        
  packages/nextjs/test/config/nextConfigToWebpackPluginConfig.test.ts
  
  
      
      
   
        
      
      
    
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              | Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| import includeAllNextjsProps, { | ||
| includeDistDir, | ||
| includeNextjsProps, | ||
| PropIncluderFn, | ||
| } from '../../src/config/nextConfigToWebpackPluginConfig'; | ||
| import { SentryWebpackPluginOptions } from '../../src/config/types'; | ||
|  | ||
| test('includeAllNextjsProps', () => { | ||
| expect(includeAllNextjsProps({ distDir: 'test' }, {})).toMatchObject({ include: 'test' }); | ||
| }); | ||
|  | ||
| describe('includeNextjsProps', () => { | ||
| const includerMap: Record<string, PropIncluderFn> = { | ||
| test: includeEverythingFn, | ||
| }; | ||
| const includeEverything = { | ||
| include: 'everything', | ||
| }; | ||
| function includeEverythingFn(): Partial<SentryWebpackPluginOptions> { | ||
| return includeEverything; | ||
| } | ||
|  | ||
| test('a prop and an includer', () => { | ||
| expect(includeNextjsProps({ test: true }, {}, includerMap, ['test'])).toMatchObject(includeEverything); | ||
| }); | ||
|  | ||
| test('a prop without includer', () => { | ||
| expect(includeNextjsProps({ noExist: false }, {}, includerMap, ['noExist'])).toMatchObject({}); | ||
| }); | ||
|  | ||
| test('an includer without a prop', () => { | ||
| expect(includeNextjsProps({ noExist: false }, {}, includerMap, ['test'])).toMatchObject({}); | ||
| }); | ||
|  | ||
| test('neither prop nor includer', () => { | ||
| expect(includeNextjsProps({}, {}, {}, [])).toMatchObject({}); | ||
| }); | ||
|  | ||
| test('duplicated props', () => { | ||
| let counter: number = 0; | ||
| const mock = jest.fn().mockImplementation(() => { | ||
| const current = counter; | ||
| counter += 1; | ||
| return { call: current }; | ||
| }); | ||
| const map: Record<string, PropIncluderFn> = { | ||
| dup: mock, | ||
| }; | ||
|  | ||
| expect(includeNextjsProps({}, {}, map, ['dup', 'dup'])).toMatchObject({ call: 0 }); | ||
| expect(mock).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
|  | ||
| describe('next config to webpack plugin config', () => { | ||
| describe('includeDistDir', () => { | ||
| const consoleWarnMock = jest.fn(); | ||
| const consoleErrorMock = jest.fn(); | ||
|  | ||
| beforeAll(() => { | ||
| global.console.warn = consoleWarnMock; | ||
| global.console.error = consoleErrorMock; | ||
| }); | ||
|  | ||
| afterAll(() => { | ||
| jest.restoreAllMocks(); | ||
| }); | ||
|  | ||
| test.each([ | ||
| [{}, {}, {}], | ||
| [{}, { include: 'path' }, { include: 'path' }], | ||
| [{}, { include: [] }, { include: [] }], | ||
| [{}, { include: ['path'] }, { include: ['path'] }], | ||
| [{}, { include: { paths: ['path'] } }, { include: { paths: ['path'] } }], | ||
| ])('without `distDir`', (nextConfig, webpackPluginConfig, expectedConfig) => { | ||
| expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig); | ||
| }); | ||
|  | ||
| test.each([ | ||
| [{ distDir: 'test' }, {}, { include: 'test' }], | ||
| [{ distDir: 'test' }, { include: 'path' }, { include: ['path', 'test'] }], | ||
| [{ distDir: 'test' }, { include: [] }, { include: ['test'] }], | ||
| [{ distDir: 'test' }, { include: ['path'] }, { include: ['path', 'test'] }], | ||
| [{ distDir: 'test' }, { include: { paths: ['path'] } }, { include: { paths: ['path', 'test'] } }], | ||
| ])('with `distDir`, different paths', (nextConfig, webpackPluginConfig, expectedConfig) => { | ||
| expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig); | ||
| }); | ||
|  | ||
| test.each([ | ||
| [{ distDir: 'path' }, { include: 'path' }, { include: 'path' }], | ||
| [{ distDir: 'path' }, { include: ['path'] }, { include: ['path'] }], | ||
| [{ distDir: 'path' }, { include: { paths: ['path'] } }, { include: { paths: ['path'] } }], | ||
| ])('with `distDir`, same path', (nextConfig, webpackPluginConfig, expectedConfig) => { | ||
| expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig); | ||
| }); | ||
|  | ||
| test.each([ | ||
| [{ distDir: 'path' }, { include: {} }, { include: { paths: ['path'] } }], | ||
| [{ distDir: 'path' }, { include: { prop: 'val' } }, { include: { prop: 'val', paths: ['path'] } }], | ||
| ])('webpack plugin config as object with other prop', (nextConfig, webpackPluginConfig, expectedConfig) => { | ||
| // @ts-ignore Other props don't match types | ||
| expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig); | ||
| expect(consoleWarnMock).toHaveBeenCalledTimes(1); | ||
| consoleWarnMock.mockClear(); | ||
| }); | ||
|  | ||
| test.each([ | ||
| [{ distDir: 'path' }, { include: { paths: {} } }, { include: { paths: {} } }], | ||
| [{ distDir: 'path' }, { include: { paths: { badObject: true } } }, { include: { paths: { badObject: true } } }], | ||
| ])('webpack plugin config as object with bad structure', (nextConfig, webpackPluginConfig, expectedConfig) => { | ||
| // @ts-ignore Bad structures don't match types | ||
| expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig); | ||
| expect(consoleErrorMock).toHaveBeenCalledTimes(1); | ||
| consoleErrorMock.mockClear(); | ||
| }); | ||
| }); | ||
| }); | 
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
Uh oh!
There was an error while loading. Please reload this page.