Skip to content
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

Pre-Bundled dependencies used over locally linked dependency when built before linking #6718

Closed
7 tasks done
MadLittleMods opened this issue Feb 2, 2022 · 10 comments · Fixed by #12790
Closed
7 tasks done
Assignees
Labels
feat: deps optimizer Esbuild Dependencies Optimization

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Feb 2, 2022

Describe the bug

Reproduction instructions

  1. Download https://stackblitz.com/edit/vitejs-vite-9ibgrz (just a base Vite project with a my-test-project directory that has a boilerplate package.json to use)
    • StackBlitz uses turbo under the hood for installing packages and I can't figure out how to do the local linking which is essential the reproduction. yarn link -> ERR!: Unknown command 'link'
  2. yarn
  3. Fake a yarn add my-test-dependency by running cp -R my-test-dependency node_modules/my-test-dependency
  4. yarn dev (this will create pre-bundled dependencies for my-test-dependency and populate node_modules/.vite)
  5. Notice my-test-dependency asdf log in the devtools console
  6. Get my-test-dependency ready for linking: cd my-test-dependency && yarn link && cd ..
  7. Locally link it in your project: yarn link my-test-dependency
  8. yarn dev
  9. Notice my-test-dependency asdf log in the devtools console
  10. Edit ./my-test-dependency/index.js to a different console.log('other message') just to show that Vite is pulling from the wrong spot
  11. Notice how the log message never changes to what you set

This use-case happens when you're doing some development on a project, find a problem in a sub-dependency, then link it locally to dive deeper.

Expected result

Vite would see that the dependency/package is now linked locally and will bust it's own pre-bundle cache.

Workaround

  1. Clear your pre-bundled dependency build cache rm -rf ./node_modules/.vite
  2. Notice how Vite is correctly bundling your locally linked package now and has the updated message

Dev notes

Relevant docs:

Reproduction

StackBlitz uses turbo under the hood for installing packages and I can't figure out how to do the local linking which is essential the reproduction. yarn link -> ERR!: Unknown command 'link'. See the reproduction steps above instead ^

System Info

  • vite@2.7.13
npx envinfo --system --npmPackages '{vite,@vitejs/*}' --binaries --browsers
npx: installed 1 in 2.131s

  System:
    OS: Windows 10 10.0.19044
  Binaries:
    Node: 14.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.17 - C:\Program Files\nodejs\yarn.CMD
    npm: 6.14.11 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 97.0.4692.99

Used Package Manager

yarn

Logs

No response

Validations

@bluwy
Copy link
Member

bluwy commented Feb 2, 2022

I have experienced this before too. This should technically not happen, since Vite's _metadata.json has an optimized field for each prebundled packages, and each package object has the src field that points to the original source code path.

export interface DepOptimizationMetadata {
/**
* The main hash is determined by user config and dependency lockfiles.
* This is checked on server startup to avoid unnecessary re-bundles.
*/
hash: string
/**
* The browser hash is determined by the main hash plus additional dependencies
* discovered at runtime. This is used to invalidate browser requests to
* optimized deps.
*/
browserHash: string
optimized: Record<
string,
{
file: string
src: string
needsInterop: boolean
}
>
}

So when linking after prebundling. The next time Vite's dev server starts it should have a different src and triggers re-prebundling. Perhaps there's a bug somewhere that incorrectly evaluating the src.

@bluwy
Copy link
Member

bluwy commented Apr 6, 2022

Hit this issue again today. I dug into the cause and it seems like it's because the hash used for cache bust is derived from the Vite config and lock file only. The fix I'd imagine is to manually resolve each dependencies and include the paths in the hash too, but now that incurs extra work for everyone who doesn't link local dependencies. I'm not really sure what's the best way here.

@patak-dev
Copy link
Member

The linking information is global, so as you said @bluwy, we would need to change the hash, or leave it as is, but add new fields to each optimized dep (maybe resolvedSrc) and check all of them to decide if we can re-use the cache. Then in browserHash include also this information (maybe it is already there because of src since it may have been resolved?).

I don't know if the extra complexity is worthy though. @MadLittleMods you can run vite with --force to recreate the cache (no need to manually calling rm to delete the cache).
Maybe we should point people to use resolutions in yarn? That works as expected now. https://classic.yarnpkg.com/lang/en/docs/selective-version-resolutions/
(or overrides in npm, pnpm.overrides in pnpm)

@MadLittleMods
Copy link
Contributor Author

I don't know if the extra complexity is worthy though. @MadLittleMods you can run vite with --force to recreate the cache (no need to manually calling rm to delete the cache).
Maybe we should point people to use resolutions in yarn? That works as expected now. https://classic.yarnpkg.com/lang/en/docs/selective-version-resolutions/
(or overrides in npm, pnpm.overrides in pnpm)

Those are fine ways to workaround the problem once you figure out the problem. But you still have to run into the pitfall in the first place and figure out why your sub-dependency isn't changing in order to utilize them.

Feels like we need some way to resolve automagically or warn and explain about what's happening.

@patak-dev
Copy link
Member

I was going to write we could at least add a warning in https://vitejs.dev/guide/dep-pre-bundling.html#monorepos-and-linked-dependencies, but we have already this:

When making changes to the linked dep, restart the dev server with the --force command line option for the changes to take effect.

It could be upgraded to a warning box maybe. A real warning may be as expensive as properly supporting the reload

@bluwy
Copy link
Member

bluwy commented Apr 6, 2022

That text is for local deps that are explicitly/manually added to optimizeDeps.include though. We don't have to add --force when making changes in the local dep by default, only when initially running npm link (hot-reloads will take effect thereafter because linked deps are excluded from optimization by default).

I think this is a tricky situation too and hard to find a balance, unless there's another trick I've not thought of.

@stagas
Copy link

stagas commented Apr 18, 2022

I'm having a similar issue I believe is related to this. I have a module published on npm and a bunch of components having it in their dependencies. Now, I discover some bug in one component but related to that module, so I link them both locally, make the change to that first module (that all the other components also depend on), and expect to see the change (because now they're linked locally). But as long as there is one component that still has the npm version of the module in its dependencies (ie not linked), then vite will pick up and use that one and completely ignore the one I changed locally. I use the vite dev server, not bundling anything at this point. Using --force and rm -rf node_modules/.vite isn't changing anything, still the same behavior.

@stagas
Copy link

stagas commented May 8, 2022

This is the code i used to solve this in a generic way, in case it's useful to anyone. It traverses all linked packages and finds all module specifiers and puts them in optimizeDeps.exclude:

const visitedPackages = new Set()
const excludeLinks = (root: string) => {
  if (visitedPackages.has(root)) return
  visitedPackages.add(root)

  try {
    const json = fs.readFileSync(path.resolve(path.join(root, 'package.json')), 'utf-8')
    const pkg = JSON.parse(json)
    for (
      const [name, version] of [
        ...Object.entries(pkg.dependencies),
        ...Object.entries(pkg.devDependencies),
      ] as [string, string][]
    ) {
      if (version.startsWith('file:')) {
        if (!optimizeDeps.exclude!.includes(name)) {
          optimizeDeps.exclude!.push(name)
          const target = path.resolve(root, version.replace('file:', ''))
          excludeLinks(target)
        }
      }
    }
  } catch {}
}
excludeLinks(root)

@AntoineParent
Copy link

This is the code i used to solve this in a generic way, in case it's useful to anyone. It traverses all linked packages and finds all module specifiers and puts them in optimizeDeps.exclude:

const visitedPackages = new Set()
const excludeLinks = (root: string) => {
  if (visitedPackages.has(root)) return
  visitedPackages.add(root)

  try {
    const json = fs.readFileSync(path.resolve(path.join(root, 'package.json')), 'utf-8')
    const pkg = JSON.parse(json)
    for (
      const [name, version] of [
        ...Object.entries(pkg.dependencies),
        ...Object.entries(pkg.devDependencies),
      ] as [string, string][]
    ) {
      if (version.startsWith('file:')) {
        if (!optimizeDeps.exclude!.includes(name)) {
          optimizeDeps.exclude!.push(name)
          const target = path.resolve(root, version.replace('file:', ''))
          excludeLinks(target)
        }
      }
    }
  } catch {}
}
excludeLinks(root)

That's sounds great.
I experience same issue.. If it works, you definitely save my day !
Where can i add this code ?

@stagas
Copy link

stagas commented May 9, 2022

That's sounds great. I experience same issue.. If it works, you definitely save my day ! Where can i add this code ?

@AntoineParent You need to pass the root from your config and have an optimizeDeps = { exclude: [] } object defined earlier so that it is populated. Then you pass them to your Vite config however you run it. See the Vite config docs for more details. Note that it might not solve for every case, it currently solves mine which is symlinking packages i own (using npm link pkg-name --save so that they get a file: url in the dependencies) and then having also dependencies that use the npm versions of those packages, work side-by-side, as they'll both use their own version when they're excluded from optimizeDeps.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: deps optimizer Esbuild Dependencies Optimization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants