Skip to content

fix(optimizer): depScan resolve with flatIdDeps#3053

Merged
patak-cat merged 1 commit intovitejs:mainfrom
nihalgonsalves:patch-1
Apr 19, 2021
Merged

fix(optimizer): depScan resolve with flatIdDeps#3053
patak-cat merged 1 commit intovitejs:mainfrom
nihalgonsalves:patch-1

Conversation

@nihalgonsalves
Copy link
Member

@nihalgonsalves nihalgonsalves commented Apr 19, 2021

Description

v2.2.0 broke my build (and other users in Vite Land) with hundreds of errors such as these:

 > node_modules/.pnpm/@material-ui+core@4.11.3_ba191c0052acfa37659f3ba04a2d9405/node_modules/@material-ui/core/es/styles/index.js:13:136: error: [vite:dep-pre-bundle] Cannot find module '@material-ui_styles'
Require stack:
- <redacted:repo-root>/node_modules/.pnpm/vite@2.2.0/node_modules/vite/dist/node/chunks/dep-8baf35b6.js
- <redacted:repo-root>/node_modules/.pnpm/vite@2.2.0/node_modules/vite/dist/node/cli.js
- <redacted:repo-root>/node_modules/.pnpm/vite@2.2.0/node_modules/vite/bin/vite.js
       13 │ export { createGenerateClassName, jssPreset, ServerStyleSheets, StylesProvider, ThemeProvider as MuiThemeProvider, ThemeProvider } from '@material-ui/styles';
          ╵                                                                                                                                         ~~~~~~~~~~~~~~~~~~~~~
   node_modules/.pnpm/vite@2.2.0/node_modules/vite/dist/node/chunks/dep-8baf35b6.js:65417:18: note: This error came from the "onResolve" callback registered here
    65417 │             build.onResolve({ filter: /^[\w@][^:]/ }, async ({ path: id, importer, kind, resolveDir }) => {
          ╵                   ~~~~~~~~~
    at setup (<redacted:repo-root>/node_modules/.pnpm/vite@2.2.0/node_modules/vite/dist/node/chunks/dep-8baf35b6.js:65417:19)
    at handlePlugins (<redacted:repo-root>/node_modules/.pnpm/esbuild@0.9.7/node_modules/esbuild/lib/main.js:686:7)
    at Object.buildOrServe (<redacted:repo-root>/node_modules/.pnpm/esbuild@0.9.7/node_modules/esbuild/lib/main.js:903:44)
    at <redacted:repo-root>/node_modules/.pnpm/esbuild@0.9.7/node_modules/esbuild/lib/main.js:1444:17
    at new Promise (<anonymous>)
    at Object.build (<redacted:repo-root>/node_modules/.pnpm/esbuild@0.9.7/node_modules/esbuild/lib/main.js:1443:14)
    at Object.build (<redacted:repo-root>/node_modules/.pnpm/esbuild@0.9.7/node_modules/esbuild/lib/main.js:1334:51)
    at optimizeDeps (<redacted:repo-root>/node_modules/.pnpm/vite@2.2.0/node_modules/vite/dist/node/chunks/dep-8baf35b6.js:65643:34)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)

I looked at the diff and it seemed to be related to this change: https://github.com/vitejs/vite/pull/3003/files#diff-7657c7c3217417a1747348410996718b471f6f0cb9552752b47a411f8f010830R94-R96

Adding back qualified[flatId] fixed it for me.

Additional context

  • Where would be the best place to add a regression test?
  • Nevertheless, this should probably be merged first since it has a big impact

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@Shinigami92
Copy link
Member

Shinigami92 commented Apr 19, 2021

Is it possible to add a test for this case?

@Shinigami92 Shinigami92 added the p3-minor-bug An edge case that only affects very specific usage (priority) label Apr 19, 2021
@patak-cat
Copy link
Member

patak-cat commented Apr 19, 2021

Let's merge this one so we can issue a release, and add tests in another PR.

@patak-cat patak-cat merged commit cb441ef into vitejs:main Apr 19, 2021
@Shinigami92 Shinigami92 mentioned this pull request Apr 19, 2021
Closed
@nihalgonsalves nihalgonsalves deleted the patch-1 branch April 21, 2021 20:58
TobiasMelen pushed a commit to TobiasMelen/vite that referenced this pull request May 3, 2021
@nihalgonsalves @TobiasMelen
fix(optimizer): depScan resolve with flatIdDeps (vitejs#3053)
45a4303
@Sociosarbis Sociosarbis mentioned this pull request May 10, 2021
Closed
@Yelmor Yelmor mentioned this pull request Jun 10, 2021
Merged
9 tasks
Yelmor added a commit to Yelmor/vite that referenced this pull request Jul 3, 2021
@Yelmor
fix: resolve nested dependencies
71a372e 
fix vitejs#3254 vitejs#4005 vitejs#4014
correct vitejs#3254 vitejs#3003 vitejs#3053
@Yelmor Yelmor mentioned this pull request Jul 4, 2021
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patak-cat patak-cat patak-cat approved these changes

+1 more reviewer

@Shinigami92 Shinigami92 Shinigami92 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

p3-minor-bug An edge case that only affects very specific usage (priority)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nihalgonsalves @Shinigami92 @patak-cat