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

isInNodeModules check is too loose #17467

Open
7 tasks done
ef4 opened this issue Jun 13, 2024 · 0 comments
Open
7 tasks done

isInNodeModules check is too loose #17467

ef4 opened this issue Jun 13, 2024 · 0 comments

Comments

@ef4
Copy link

ef4 commented Jun 13, 2024

Describe the bug

If the path to your project contains the substring "node_modules", many vite features break. The simplest one to demonstrate is that inline script tags in HTML break.

My reproduction below is the stock vite vanilla blueprint with a single inline script tag added to index.html. All that's needed to reproduce the bug is to name the project "node_modules_bug".

In the specific case of the inline script tag, the failure happens because ensureVersionQuery incorrectly decides that index.html?html-proxy&index=0.js is in node_modules and rewrites it to index.html?v=af416d6b&html-proxy&index=0.js, which then fails to match the regex in html-inline-proxy, because that regex only works when html-proxy is the first search param in the URL.

At a minimum, isInNodeModules should probably require path separators on both sides of the node_modules string. This would fix my specific reproduction, but would still leave people broken who happen to have a project root with an ancestor directory named "node_modules" (which is definitely a thing sometimes when dealing with legacy codebases).

A complete fix would make isInNodeModules aware of the configured root, because whether or not a given path comes into your project via node_modules is ultimately a question relative to that root:

Given: 

  root=/path/to/node_modules/your-project

Then:

  inNodeModules("/path/to/node_modules/your-project/x", root) -> false because x is clearly in our configured root
  inNodeModules("/path/to/node_modules/your-project/node_modules/y", root) -> true because we left the configured root "inward"
  inNodeModules("/path/to/node_modules/z", root) -> true because we left the configured root "outward"

Reproduction

https://github.com/ef4/node_modules_bug

Steps to reproduce

  1. Clone the above repo (using the default directory naming, which will put the project in a directory named "node_modules_bug").
  2. pnpm install
  3. pnpm vite dev

System Info

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M1 Max
    Memory: 4.20 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.14.0 - /usr/local/bin/node
    npm: 10.7.0 - /usr/local/bin/npm
    pnpm: 8.15.8 - /usr/local/bin/pnpm
    Watchman: 2023.02.13.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 125.0.6422.142
    Safari: 17.5
  npmPackages:
    vite: ^5.2.0 => 5.3.0

Used Package Manager

pnpm

Logs

Click to expand!
[~/hacking/node_modules_bug]$ pnpm vite dev --debug                                                                                                                                      [main][ruby-2.7.4]
  vite:config no config file found. +0ms
  vite:config using resolved config: {
  vite:config   root: '/Users/edward/hacking/node_modules_bug',
  vite:config   base: '/',
  vite:config   mode: 'development',
  vite:config   configFile: undefined,
  vite:config   logLevel: undefined,
  vite:config   clearScreen: undefined,
  vite:config   optimizeDeps: {
  vite:config     holdUntilCrawlEnd: true,
  vite:config     force: undefined,
  vite:config     esbuildOptions: { preserveSymlinks: false }
  vite:config   },
  vite:config   server: {
  vite:config     preTransformRequests: true,
  vite:config     host: undefined,
  vite:config     sourcemapIgnoreList: [Function: isInNodeModules$1],
  vite:config     middlewareMode: false,
  vite:config     fs: {
  vite:config       strict: true,
  vite:config       allow: [Array],
  vite:config       deny: [Array],
  vite:config       cachedChecks: undefined
  vite:config     }
  vite:config   },
  vite:config   configFileDependencies: [],
  vite:config   inlineConfig: {
  vite:config     root: undefined,
  vite:config     base: undefined,
  vite:config     mode: undefined,
  vite:config     configFile: undefined,
  vite:config     logLevel: undefined,
  vite:config     clearScreen: undefined,
  vite:config     optimizeDeps: { force: undefined },
  vite:config     server: { host: undefined }
  vite:config   },
  vite:config   rawBase: '/',
  vite:config   resolve: {
  vite:config     mainFields: [ 'browser', 'module', 'jsnext:main', 'jsnext' ],
  vite:config     conditions: [],
  vite:config     extensions: [
  vite:config       '.mjs',  '.js',
  vite:config       '.mts',  '.ts',
  vite:config       '.jsx',  '.tsx',
  vite:config       '.json'
  vite:config     ],
  vite:config     dedupe: [],
  vite:config     preserveSymlinks: false,
  vite:config     alias: [ [Object], [Object] ]
  vite:config   },
  vite:config   publicDir: '/Users/edward/hacking/node_modules_bug/public',
  vite:config   cacheDir: '/Users/edward/hacking/node_modules_bug/node_modules/.vite',
  vite:config   command: 'serve',
  vite:config   ssr: {
  vite:config     target: 'node',
  vite:config     optimizeDeps: { noDiscovery: true, esbuildOptions: [Object] }
  vite:config   },
  vite:config   isWorker: false,
  vite:config   mainConfig: null,
  vite:config   bundleChain: [],
  vite:config   isProduction: false,
  vite:config   plugins: [
  vite:config     'vite:optimized-deps',
  vite:config     'vite:watch-package-data',
  vite:config     'vite:pre-alias',
  vite:config     'alias',
  vite:config     'vite:modulepreload-polyfill',
  vite:config     'vite:resolve',
  vite:config     'vite:html-inline-proxy',
  vite:config     'vite:css',
  vite:config     'vite:esbuild',
  vite:config     'vite:json',
  vite:config     'vite:wasm-helper',
  vite:config     'vite:worker',
  vite:config     'vite:asset',
  vite:config     'vite:wasm-fallback',
  vite:config     'vite:define',
  vite:config     'vite:css-post',
  vite:config     'vite:worker-import-meta-url',
  vite:config     'vite:asset-import-meta-url',
  vite:config     'vite:dynamic-import-vars',
  vite:config     'vite:import-glob',
  vite:config     'vite:client-inject',
  vite:config     'vite:css-analysis',
  vite:config     'vite:import-analysis'
  vite:config   ],
  vite:config   css: { lightningcss: undefined },
  vite:config   esbuild: { jsxDev: true },
  vite:config   build: {
  vite:config     target: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari14' ],
  vite:config     cssTarget: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari14' ],
  vite:config     outDir: 'dist',
  vite:config     assetsDir: 'assets',
  vite:config     assetsInlineLimit: 4096,
  vite:config     cssCodeSplit: true,
  vite:config     sourcemap: false,
  vite:config     rollupOptions: {},
  vite:config     minify: 'esbuild',
  vite:config     terserOptions: {},
  vite:config     write: true,
  vite:config     emptyOutDir: null,
  vite:config     copyPublicDir: true,
  vite:config     manifest: false,
  vite:config     lib: false,
  vite:config     ssr: false,
  vite:config     ssrManifest: false,
  vite:config     ssrEmitAssets: false,
  vite:config     reportCompressedSize: true,
  vite:config     chunkSizeWarningLimit: 500,
  vite:config     watch: null,
  vite:config     commonjsOptions: { include: [Array], extensions: [Array] },
  vite:config     dynamicImportVarsOptions: { warnOnError: true, exclude: [Array] },
  vite:config     modulePreload: { polyfill: true },
  vite:config     cssMinify: true
  vite:config   },
  vite:config   preview: {
  vite:config     port: undefined,
  vite:config     strictPort: undefined,
  vite:config     host: undefined,
  vite:config     https: undefined,
  vite:config     open: undefined,
  vite:config     proxy: undefined,
  vite:config     cors: undefined,
  vite:config     headers: undefined
  vite:config   },
  vite:config   envDir: '/Users/edward/hacking/node_modules_bug',
  vite:config   env: { BASE_URL: '/', MODE: 'development', DEV: true, PROD: false },
  vite:config   assetsInclude: [Function: assetsInclude],
  vite:config   logger: {
  vite:config     hasWarned: false,
  vite:config     info: [Function: info],
  vite:config     warn: [Function: warn],
  vite:config     warnOnce: [Function: warnOnce],
  vite:config     error: [Function: error],
  vite:config     clearScreen: [Function: clearScreen],
  vite:config     hasErrorLogged: [Function: hasErrorLogged]
  vite:config   },
  vite:config   packageCache: Map(1) {
  vite:config     'fnpd_/Users/edward/hacking/node_modules_bug' => {
  vite:config       dir: '/Users/edward/hacking/node_modules_bug',
  vite:config       data: [Object],
  vite:config       hasSideEffects: [Function: hasSideEffects],
  vite:config       webResolvedImports: {},
  vite:config       nodeResolvedImports: {},
  vite:config       setResolvedCache: [Function: setResolvedCache],
  vite:config       getResolvedCache: [Function: getResolvedCache]
  vite:config     },
  vite:config     set: [Function (anonymous)]
  vite:config   },
  vite:config   createResolver: [Function: createResolver],
  vite:config   worker: { format: 'iife', plugins: '() => plugins', rollupOptions: {} },
  vite:config   appType: 'spa',
  vite:config   experimental: { importGlobRestoreExtension: false, hmrPartialAccept: false },
  vite:config   getSortedPlugins: [Function: getSortedPlugins],
  vite:config   getSortedPluginHooks: [Function: getSortedPluginHooks]
  vite:config } +4ms
  vite:deps Hash is consistent. Skipping. Use --force to override. +0ms

  VITE v5.3.0  ready in 98 ms

  ➜  Local:   http://localhost:5173/
  ➜  Network: use --host to expose
  ➜  press h + enter to show help
  vite:time 2.51ms / +0ms
  vite:html-fallback Rewriting GET / to /index.html +0ms
  vite:time 23.13ms /index.html +30ms
  vite:resolve 4.22ms /main.js -> /Users/edward/hacking/node_modules_bug/main.js?v=af416d6b +0ms
  vite:resolve 3.04ms /index.html?html-proxy&index=0.js -> /Users/edward/hacking/node_modules_bug/index.html?v=af416d6b&html-proxy&index=0.js +0ms
  vite:load 2.26ms [fs] /main.js +0ms
  vite:resolve 0.76ms ./style.css -> /Users/edward/hacking/node_modules_bug/style.css +7ms
  vite:resolve 0.80ms ./javascript.svg -> /Users/edward/hacking/node_modules_bug/javascript.svg +0ms
  vite:resolve 0.72ms ./counter.js -> /Users/edward/hacking/node_modules_bug/counter.js?v=af416d6b +0ms
  vite:import-analysis 2.49ms [4 imports rewritten] main.js?v=af416d6b +0ms
  vite:transform 4.75ms /main.js +0ms
  vite:load 0.54ms [plugin] /javascript.svg +6ms
  vite:load 0.68ms [plugin] /vite.svg +0ms
  vite:import-analysis 0.15ms [no imports] javascript.svg +1ms
  vite:import-analysis 0.01ms [no imports] /vite.svg +0ms
  vite:transform 0.63ms /javascript.svg +1ms
  vite:transform 0.62ms /vite.svg +0ms
  vite:load 8.99ms [fs] /index.html?html-proxy&index=0.js +1ms
9:52:45 AM [vite] Pre-transform error: Failed to parse source for import analysis because the content contains invalid JS syntax. You may need to install appropriate plugins to handle the .js file format, or if it's an asset, add "**/*.js" to `assetsInclude` in your configuration.
  vite:resolve 0.16ms /@vite/client -> /Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/client.mjs +22ms
  vite:cache [memory] /main.js +0ms
  vite:time 0.64ms /main.js +32ms
  vite:load 22.54ms [fs] /counter.js?v=af416d6b +21ms
  vite:import-analysis 0.09ms [no imports] counter.js?v=af416d6b +22ms
  vite:transform 0.31ms /counter.js?v=af416d6b +22ms
  vite:load 23.21ms [fs] /style.css?v=af416d6b +0ms
  vite:hmr [self-accepts] style.css +0ms
  vite:import-analysis 0.42ms [0 imports rewritten] style.css +1ms
  vite:transform 1.26ms /style.css?v=af416d6b +2ms
  vite:load 4.15ms [fs] /index.html?html-proxy&index=0.js +2ms
9:52:45 AM [vite] Internal server error: Failed to parse source for import analysis because the content contains invalid JS syntax. You may need to install appropriate plugins to handle the .js file format, or if it's an asset, add "**/*.js" to `assetsInclude` in your configuration.
  Plugin: vite:import-analysis
  File: /Users/edward/hacking/node_modules_bug/index.html?v=af416d6b&html-proxy&index=0.js:7:28
  5  |      <link rel="icon" type="image/svg+xml" href="/vite.svg" />
  6  |      <meta name="viewport" content="width=device-width, initial-scale=1.0" />
  7  |      <title>Vite App</title>
     |                             ^
  8  |    </head>
  9  |    <body>
      at TransformPluginContext._formatError (file:///Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-h78lQ5BT.js:49703:41)
      at TransformPluginContext.error (file:///Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-h78lQ5BT.js:49698:16)
      at TransformPluginContext.transform (file:///Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-h78lQ5BT.js:64212:14)
      at async PluginContainer.transform (file:///Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-h78lQ5BT.js:49544:18)
      at async loadAndTransform (file:///Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-h78lQ5BT.js:52366:27)
      at async viteTransformMiddleware (file:///Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-h78lQ5BT.js:62102:24)
  vite:time 8.16ms /index.html?html-proxy&index=0.js +7ms
  vite:load 7.62ms [fs] /@vite/client +4ms
  vite:resolve 0.06ms @vite/env -> /Users/edward/hacking/node_modules_bug/node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/env.mjs +8ms
  vite:import-analysis 0.55ms [1 imports rewritten] node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/client.mjs +6ms
  vite:transform 0.77ms /@vite/client +5ms
  vite:time 8.90ms /@vite/client +1ms
  vite:cache [memory] /style.css?v=af416d6b +8ms
  vite:time 0.08ms /style.css?v=af416d6b +0ms
  vite:cache [memory] /counter.js?v=af416d6b +1ms
  vite:time 0.27ms /counter.js?v=af416d6b +1ms
  vite:cache [304] /javascript.svg?import +0ms
  vite:time 0.04ms /javascript.svg?import +1ms
  vite:load 2.54ms [fs] /node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/env.mjs +3ms
  vite:import-analysis 0.02ms [no imports] node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/env.mjs +2ms
  vite:transform 0.29ms /node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/env.mjs +2ms
  vite:cache [304] /node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/env.mjs +1ms
  vite:time 0.03ms /node_modules/.pnpm/[email protected]/node_modules/vite/dist/client/env.mjs +1ms
  vite:cache [304] /vite.svg?import +0ms
  vite:time 0.02ms /vite.svg?import +0ms
  vite:time 1.74ms /javascript.svg +5ms
  vite:time 1.14ms /vite.svg +1ms
  vite:time 0.12ms /vite.svg +3ms

Validations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant