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

Conditional Export Resolves Incorrectly For Browser After Previously Loading For SSR #4968

Closed
7 tasks done
wlib opened this issue Sep 18, 2021 · 4 comments
Closed
7 tasks done

Comments

@wlib
Copy link

wlib commented Sep 18, 2021

Describe the bug

I am encountering this bug where after resolving an conditional export for node in SSR, that same conditional export will incorrectly load the node version in a browser script. For some reason, this only happens in this one case during dev mode.

Reproduction

I tried to make a simpler reproduction but I only find the bug appearing in this case, and it is simple enough to replicate.
FYI for full transparency, this will run this npm init script which creates a vite project which uses this vite plugin and the offending conditional exports can be found in this package.json.

npm init bruh@0.5.2
# Choose "vite"
# Follow prompts any way
npm run dev # This command will succeed, but the error is visible in the browser's devtools
npm run build && npm run serve # This will work as intended, no bug here

System Info

System:
    OS: Linux 5.10 NixOS 21.11 (Porcupine) 21.11pre293089.1c2986bbb80 (Porcupine)
    CPU: (12) x64 AMD Ryzen 5 1600 Six-Core Processor
    Memory: 389.68 MB / 15.65 GB
    Container: Yes
    Shell: 3.2.2 - /run/current-system/sw/bin/fish
  Binaries:
    Node: 16.2.0 - /run/current-system/sw/bin/node
    Yarn: 1.22.10 - ~/.nix-profile/bin/yarn
    npm: 7.13.0 - /run/current-system/sw/bin/npm
  npmPackages:
    vite: ^2.5.1 => 2.5.1

Used Package Manager

npm

Logs

npx vite --debug
  vite:config native esm config loaded in 223ms URL {
  href: 'file:///home/daniel/Development/buggy/vite.config.mjs',
  origin: 'null',
  protocol: 'file:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: '/home/daniel/Development/buggy/vite.config.mjs',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
} +0ms
  vite:config using resolved config: {
  vite:config   plugins: [
  vite:config     'vite:pre-alias',
  vite:config     'alias',
  vite:config     'bruh-mdx',
  vite:config     'bruh-dev',
  vite:config     'vite:modulepreload-polyfill',
  vite:config     'vite:resolve',
  vite:config     'vite:html',
  vite:config     'vite:css',
  vite:config     'vite:esbuild',
  vite:config     'vite:json',
  vite:config     'vite:wasm',
  vite:config     'vite:worker',
  vite:config     'vite:asset',
  vite:config     'bruh-jsx',
  vite:config     'vite:define',
  vite:config     'vite:css-post',
  vite:config     'vite:client-inject',
  vite:config     'vite:import-analysis'
  vite:config   ],
  vite:config   server: { fs: { strict: undefined, allow: [Array] } },
  vite:config   ssr: { external: [ 'fs', 'path', 'crypto' ] },
  vite:config   esbuild: {
  vite:config     jsxFactory: 'h',
  vite:config     jsxFragment: 'JSXFragment',
  vite:config     jsxInject: 'import { h, JSXFragment } from "bruh/dom"'
  vite:config   },
  vite:config   configFile: '/home/daniel/Development/buggy/vite.config.mjs',
  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     server: { fs: [Object] }
  vite:config   },
  vite:config   root: '/home/daniel/Development/buggy',
  vite:config   base: '/',
  vite:config   resolve: { dedupe: undefined, alias: [ [Object], [Object] ] },
  vite:config   publicDir: '/home/daniel/Development/buggy/public',
  vite:config   cacheDir: '/home/daniel/Development/buggy/node_modules/.vite',
  vite:config   command: 'serve',
  vite:config   mode: 'development',
  vite:config   isProduction: false,
  vite:config   build: {
  vite:config     target: [ 'es2019', 'edge88', 'firefox78', 'chrome87', 'safari13.1' ],
  vite:config     polyfillModulePreload: true,
  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     commonjsOptions: { include: [Array], extensions: [Array] },
  vite:config     dynamicImportVarsOptions: { warnOnError: true, exclude: [Array] },
  vite:config     minify: 'terser',
  vite:config     terserOptions: {},
  vite:config     write: true,
  vite:config     emptyOutDir: null,
  vite:config     manifest: false,
  vite:config     lib: false,
  vite:config     ssr: false,
  vite:config     ssrManifest: false,
  vite:config     brotliSize: true,
  vite:config     chunkSizeWarningLimit: 500,
  vite:config     watch: null
  vite:config   },
  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   createResolver: [Function: createResolver],
  vite:config   optimizeDeps: { esbuildOptions: { keepNames: undefined } }
  vite:config } +7ms
Could not determine entry point from rollupOptions or html files. Skipping dependency pre-bundling.
  vite:deps No dependencies to bundle. Skipping.
  vite:deps 
  vite:deps 
  vite:deps  +0ms

  vite v2.5.1 dev server running at:

  > Local: http://localhost:3000/
  > Network: use `--host` to expose

  ready in 409ms.

  vite:load 0ms   [fs] index.html.jsx +0ms
  vite:resolve 1ms   bruh/dom -> /home/daniel/Development/buggy/node_modules/bruh/src/dom/node/index.mjs +0ms
  vite:resolve 0ms   ./shell -> /home/daniel/Development/buggy/shell.jsx +2ms
  vite:resolve 0ms   ./components/counter/render -> /home/daniel/Development/buggy/components/counter/render.jsx +0ms
  vite:resolve 0ms   /node_modules/bruh/src/dom/node/index.mjs -> /home/daniel/Development/buggy/node_modules/bruh/src/dom/node/index.mjs +1ms
  vite:resolve 0ms   /shell.jsx -> /home/daniel/Development/buggy/shell.jsx +0ms
  vite:resolve 0ms   /components/counter/render.jsx -> /home/daniel/Development/buggy/components/counter/render.jsx +0ms
  vite:transform 19ms  index.html.jsx +0ms
  vite:load 1ms   [fs] /node_modules/bruh/src/dom/node/index.mjs +36ms
  vite:rewrite 0ms   [no imports] node_modules/bruh/src/dom/node/index.mjs +0ms
  vite:transform 1ms   /node_modules/bruh/src/dom/node/index.mjs +17ms
  vite:load 0ms   [fs] /shell.jsx +65ms
  vite:transform 3ms   /shell.jsx +67ms
  vite:load 1ms   [fs] /components/counter/render.jsx +23ms
  vite:transform 2ms   /components/counter/render.jsx +22ms
  vite:resolve 0ms   /@vite/client -> /home/daniel/Development/buggy/node_modules/vite/dist/client/client.mjs +163ms
  vite:load 1ms   [fs] /@vite/client +61ms
  vite:resolve 1ms   @vite/env -> /home/daniel/Development/buggy/node_modules/vite/dist/client/env.mjs +8ms
  vite:resolve 0ms   /node_modules/vite/dist/client/env.mjs -> /home/daniel/Development/buggy/node_modules/vite/dist/client/env.mjs +1ms
  vite:transform 6ms   /@vite/client +67ms
  vite:time 13ms  /@vite/client +0ms
  vite:load 1ms   [fs] /node_modules/vite/dist/client/env.mjs +20ms
  vite:rewrite 2ms   [no imports] node_modules/vite/dist/client/env.mjs +170ms
  vite:transform 3ms   /node_modules/vite/dist/client/env.mjs +15ms
  vite:time 4ms   /node_modules/vite/dist/client/env.mjs +14ms
10:12:49 PM [vite] new dependencies found: bruh/dom, updating...
  vite:deps deps bundled in 8ms +2s
10:12:49 PM [vite] ✨ dependencies updated, reloading page...
  vite:load 0ms   [fs] index.html.jsx +59ms
  vite:transform 2ms   index.html.jsx +59ms
  vite:load 1ms   [fs] /node_modules/bruh/src/dom/node/index.mjs +6ms
  vite:rewrite 0ms   [no imports] node_modules/bruh/src/dom/node/index.mjs +64ms
  vite:transform 0ms   /node_modules/bruh/src/dom/node/index.mjs +4ms
  vite:load 0ms   [fs] /shell.jsx +20ms
  vite:transform 2ms   /shell.jsx +21ms
  vite:load 0ms   [fs] /components/counter/render.jsx +21ms
  vite:transform 2ms   /components/counter/render.jsx +21ms
  vite:resolve 0ms   /index.mjs -> /home/daniel/Development/buggy/index.mjs +146ms
  vite:load 1ms   [fs] /@vite/client +27ms
  vite:transform 3ms   /@vite/client +29ms
  vite:time 7ms   /@vite/client +134ms
  vite:load 5ms   [fs] /index.mjs +5ms
  vite:resolve 0ms   ./index.css -> /home/daniel/Development/buggy/index.css +6ms
  vite:resolve 0ms   /index.css -> /home/daniel/Development/buggy/index.css +0ms
  vite:resolve 0ms   ./components/counter/hydrate.mjs -> /home/daniel/Development/buggy/components/counter/hydrate.mjs +1ms
  vite:resolve 2ms   /components/counter/hydrate.mjs -> /home/daniel/Development/buggy/components/counter/hydrate.mjs +2ms
  vite:transform 5ms   /index.mjs +6ms
  vite:time 11ms  /index.mjs +6ms
  vite:load 0ms   [fs] /node_modules/vite/dist/client/env.mjs +8ms
  vite:rewrite 0ms   [no imports] node_modules/vite/dist/client/env.mjs +80ms
  vite:transform 1ms   /node_modules/vite/dist/client/env.mjs +4ms
  vite:time 2ms   /node_modules/vite/dist/client/env.mjs +4ms
  vite:load 0ms   [fs] /index.css +4ms
  vite:load 4ms   [fs] /components/counter/hydrate.mjs +9ms
  vite:resolve 1ms   bruh/dom -> /home/daniel/Development/buggy/node_modules/.vite/bruh_dom.js?v=8a1cc1f8 +18ms
  vite:resolve 0ms   /node_modules/.vite/bruh_dom.js?v=8a1cc1f8 -> /home/daniel/Development/buggy/node_modules/.vite/bruh_dom.js?v=8a1cc1f8 +0ms
  vite:transform 1ms   /components/counter/hydrate.mjs +13ms
  vite:time 6ms   /components/counter/hydrate.mjs +13ms
  vite:hmr [self-accepts] index.css +0ms
  vite:transform 547ms /index.css +538ms
  vite:time 549ms /index.css +538ms
  vite:load 0ms   [fs] /node_modules/.vite/bruh_dom.js?v=8a1cc1f8 +541ms
  vite:rewrite 0ms   [no imports] node_modules/.vite/bruh_dom.js?v=8a1cc1f8 +555ms
  vite:transform 0ms   /node_modules/.vite/bruh_dom.js?v=8a1cc1f8 +3ms
  vite:time 3ms   /node_modules/.vite/bruh_dom.js?v=8a1cc1f8 +4ms
  vite:time 1058ms /index.mjs +346ms

Validations

@wlib
Copy link
Author

wlib commented Sep 19, 2021

I'm attempting to debug this right now, and I think I found a really solid lead.

server._optimizeDepsMetadata.optimized is an object that looks like this:

{
  "bruh/dom": {
    "file": "/home/daniel/Development/buggy/node_modules/.vite/bruh_dom.js",
    "src": "/home/daniel/Development/buggy/node_modules/bruh/src/dom/node/index.mjs",
    "needsInterop": false
  }
}

It seems that this object maps from the import to a previously vite-transformed version - but there is no specifier of the export condition that leads the import "bruh/dom" to its src.

Where this goes wrong is when an assumption is made that "bruh/dom" was imported with the same condition (leading to the same file) before as it is now. So when it was resolved to the node version earlier for SSR, it got stored in this object. Then when the import analysis was conducted for some browser code, and it was found to import "bruh/dom", it thought it could just short-circuit to use the previously transformed file.

Here is the story, roughly as I see it from my debugging session:

  1. Load index.html.jsx with ssrLoadModule() in my plugin in order to dynamically create an index.html file
  2. In this process of loading the jsx file that is made to run in node, resolve the node conditional export for "bruh/dom"
  3. Store this node version of "bruh/dom" in server._optimizeDepsMetadata.optimized to avoid extra work later
  4. Once the index.html.jsx file is run in my plugin to generate a virtual index.html file, the plugin uses transformIndexHtml() to let vite process the browser-side imports
  5. After a couple other successful things, we get to the interesting part where the bug happens...
  6. vite:import-analysis's transform hook calls this.resolve("bruh/dom", "/home/daniel/Development/buggy/components/counter/hydrate.mjs")
  7. That calls (within the plugin container) container.resolveId with the two key arguments above
  8. That ends up calling vite:pre-alias's resolveId hook
  9. That calls tryOptimizedResolve("bruh/dom", server) - note that the dev server is the same one that was used earlier to generate the virtual index.html, and it previously resolved "bruh/dom" to its node conditional export
  10. tryOptimizedResolve uses the server._optimizeDepsMetadata.optimized object and sees this node version, even though it should have ignored it and looked for a browser version, given that it is trying to resolve "bruh/dom" for a browser-side import

It looks like the source of the problem was the assumption that a vite dev server won't have to worry about different import conditions. I fixed this bug by simply making tryOptimizedResolve always return nothing, but obviously the correct solution is to branch server._optimizeDepsMetadata.optimized by export condition (which for vite right now would just be "node" vs "browser"). I will work on this and make a PR when I can.

Quick note: my proposal for a generalized ssrLoadModule would have probably made this bug visible earlier, as I only discovered it by bending vite a little bit to work in that proposed way for my plugin that allows dynamic entry html files.

@bluwy
Copy link
Member

bluwy commented Sep 23, 2021

@wlib This should be fixed in #5017, it's not released yet but you can try it out locally at the meantime.

@wlib
Copy link
Author

wlib commented Oct 7, 2021

@bluwy @ygj6 Yes, it looks like #5017 solved this! One thing that it potentially regressed, however -> https://github.com/vitejs/vite/pull/5017/files#r723797042

@Niputi
Copy link
Contributor

Niputi commented Dec 5, 2021

closing as #4968 (comment) should be resolved with #5248 in 2.6.11

improved version #5827 in 2.7.0-beta.9

@Niputi Niputi closed this as completed Dec 5, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants