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

Works in Dev, Not in Prod - imported module has undefined exports #10612

Closed
7 tasks done
KevinBrogan opened this issue Oct 24, 2022 · 2 comments
Closed
7 tasks done

Works in Dev, Not in Prod - imported module has undefined exports #10612

KevinBrogan opened this issue Oct 24, 2022 · 2 comments
Labels
feat: commonjs @rollup/plugin-commonjs issue has workaround inconsistency Inconsistency between dev & build

Comments

@KevinBrogan
Copy link

Describe the bug

I am importing a package that has been installed via npm.
Development works fine
The package imports are undefined in production

Reproduction

https://stackblitz.com/edit/vitejs-vite-vtgp1g?file=main.js

Steps to reproduce

vite build
vite preview

check console in preview window and see that the logged module is empty
Note how dev had the module contents

System Info

stackblitz

❯ npx envinfo --system --npmPackages '{vite,@vitejs/*}' --binaries --browsers
success Install finished in 1.074s

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    vite: ^3.1.8 => 3.1.8

Used Package Manager

npm

Logs

Click to expand!
❯ vite build --debug
  vite:config no config file found. +0ms
  vite:esbuild init tsconfck (root: /home/projects/vitejs-vite-vtgp1g) +0ms
  vite:esbuild init tsconfck (root: /home/projects/vitejs-vite-vtgp1g) +1ms
  vite:esbuild init tsconfck (root: /home/projects/vitejs-vite-vtgp1g) +1ms
  vite:esbuild init tsconfck (root: /home/projects/vitejs-vite-vtgp1g) +0ms
  vite:esbuild init tsconfck end +2ms
  vite:esbuild init tsconfck end +0ms
  vite:esbuild init tsconfck end +0ms
  vite:esbuild init tsconfck end +0ms
  vite:config using resolved config: {
  vite:config   root: '/home/projects/vitejs-vite-vtgp1g',
  vite:config   base: '/',
  vite:config   mode: 'production',
  vite:config   configFile: undefined,
  vite:config   logLevel: undefined,
  vite:config   clearScreen: undefined,
  vite:config   optimizeDeps: {
  vite:config     disabled: 'build',
  vite:config     force: undefined,
  vite:config     esbuildOptions: { preserveSymlinks: undefined }
  vite:config   },
  vite:config   build: {
  vite:config     target: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari13' ],
  vite:config     polyfillModulePreload: true,
  vite:config     outDir: 'dist',
  vite:config     assetsDir: 'assets',
  vite:config     assetsInlineLimit: 4096,
  vite:config     cssCodeSplit: true,
  vite:config     cssTarget: [ 'es2020', 'edge88', 'firefox78', 'chrome87', 'safari13' ],
  vite:config     sourcemap: false,
  vite:config     rollupOptions: {},
  vite:config     minify: 'esbuild',
  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     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   },
  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     build: {}
  vite:config   },
  vite:config   resolve: { alias: [ [Object], [Object] ] },
  vite:config   publicDir: '/home/projects/vitejs-vite-vtgp1g/public',
  vite:config   cacheDir: '/home/projects/vitejs-vite-vtgp1g/node_modules/.vite',
  vite:config   command: 'build',
  vite:config   ssr: {
  vite:config     format: 'esm',
  vite:config     target: 'node',
  vite:config     optimizeDeps: { disabled: true, esbuildOptions: [Object] }
  vite:config   },
  vite:config   isWorker: false,
  vite:config   mainConfig: null,
  vite:config   isProduction: true,
  vite:config   plugins: [
  vite:config     'vite:build-metadata',
  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:build-html',
  vite:config     'vite:worker-import-meta-url',
  vite:config     'vite:force-systemjs-wrap-complete',
  vite:config     'vite:watch-package-data',
  vite:config     'commonjs',
  vite:config     'vite:data-uri',
  vite:config     'vite:asset-import-meta-url',
  vite:config     'vite:dynamic-import-vars',
  vite:config     'vite:import-glob',
  vite:config     'vite:build-import-analysis',
  vite:config     'vite:esbuild-transpile',
  vite:config     'vite:terser',
  vite:config     'vite:reporter',
  vite:config     'vite:load-fallback'
  vite:config   ],
  vite:config   server: {
  vite:config     preTransformRequests: true,
  vite:config     middlewareMode: false,
  vite:config     fs: { strict: true, allow: [Array], deny: [Array] }
  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   env: { BASE_URL: '/', MODE: 'production', DEV: false, PROD: true },
  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(0) { set: [Function (anonymous)] },
  vite:config   createResolver: [Function: createResolver],
  vite:config   worker: {
  vite:config     format: 'iife',
  vite:config     plugins: [
  vite:config       'vite:build-metadata',
  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:build-html',
  vite:config       'vite:worker-import-meta-url',
  vite:config       'vite:force-systemjs-wrap-complete',
  vite:config       'vite:watch-package-data',
  vite:config       'commonjs',
  vite:config       'vite:data-uri',
  vite:config       'vite:asset-import-meta-url',
  vite:config       'vite:dynamic-import-vars',
  vite:config       'vite:import-glob',
  vite:config       'vite:build-import-analysis',
  vite:config       'vite:esbuild-transpile',
  vite:config       'vite:terser',
  vite:config       'vite:reporter',
  vite:config       'vite:load-fallback'
  vite:config     ],
  vite:config     rollupOptions: {},
  vite:config     getSortedPlugins: [Function: getSortedPlugins],
  vite:config     getSortedPluginHooks: [Function: getSortedPluginHooks]
  vite:config   },
  vite:config   appType: 'spa',
  vite:config   experimental: { importGlobRestoreExtension: false, hmrPartialAccept: false },
  vite:config   getSortedPlugins: [Function: getSortedPlugins],
  vite:config   getSortedPluginHooks: [Function: getSortedPluginHooks]
  vite:config } +10ms
vite v3.1.8 building for production...
✓ 9 modules transformed.
dist/index.html                 0.22 KiB
dist/assets/index.150a79b3.js   3.18 KiB / gzip: 1.60 KiB

Validations

@sapphi-red sapphi-red added inconsistency Inconsistency between dev & build feat: commonjs @rollup/plugin-commonjs issue labels Oct 24, 2022
@Tanimodori
Copy link

Tanimodori commented Apr 2, 2023

Checkek out https://stackblitz.com/edit/vitejs-vite-nlwsq9?file=vite.config.js

The old siphash has something incompatible with bundlers here:

node_modules\siphash\lib\siphash.js

var SipHash = (function () {
    // ...
    return {
        hash: hash,
        hash_hex: hash_hex,
        hash_uint: hash_uint,
        string16_to_key: string16_to_key,
        string_to_u8: string_to_u8
    };
})();
var module = module || {}, exports = module.exports = SipHash;

The var module = module || {}, here is problematic, which let the bundlers to think that module is a regular variable rather than module exports. Remove the line results in successful build. You can use either rollup transform or patch-package to do the job.

vite.config.js

import { defineConfig } from "vite";

export default defineConfig({
  plugins: [
    {
      name: "fix-siphash-exports",
      transform(code, id) {
        const siphashRE = /node_modules\/siphash\/lib\/.*\.js$/;
        if(siphashRE.test(id)) {
          return code.replace("var module = module || {},", "");
        }
      }
    }
  ]
})

@bluwy
Copy link
Member

bluwy commented Oct 29, 2023

I tried to fix what Tanimodori explained today. Ultimately, it introduces quite some complexity and given that it's a really edge case, I don't think this is worth fixing. The library should try to avoid the syntax, especially that module.exports directly would've already worked fine.

I'm going to close this as a wontfix, but if anyone is interested in fixing still, here's the rundown of where to fix it.


As shown, var module = module || {}, exports = module.exports = SipHash; causes an issue, where the second module is referring to the global module reference, however @rollup/plugin-commonjs wrongly analyzed that it's referring to var module.

This scope analysis is done here in @rollup/plugin-commonjs:

https://github.com/rollup/plugins/blob/4f9fc5a66eac9e6183ec2118caeadd57408a8f30/packages/commonjs/src/transform-commonjs.js#L294

The scope is created through its attachScopes util at:

https://github.com/rollup/plugins/blob/4f9fc5a66eac9e6183ec2118caeadd57408a8f30/packages/pluginutils/src/attachScopes.ts#L61

The utility assigns AST nodes that creates a scope with .scope. This is used by @rollup/plugin-commonjs. To fix this, you need to create another scope for the right-hand-side of var/let/const statements that excludes the variable name as part of the scope. HOWEVER, not for functions/classes in the right-hand side, they could have a different execution order.

// Example case

var module = module || (() => module)(); other()
//           |--------|
//         new scope here (A*)
//                     |--------------|
//                new scope but inherits from A (the existing code would take
//                A* which is incorrect, this needs to be specially handled)
// |-------------------------------------------|
//                parent scope (A)

A* is the new special scope we need to create. In practice, scopes don't really work like that? And it's kinda a special-case "scope" to only fix this issue. So altogether I didn't feel it's worth the effort.

@bluwy bluwy closed this as not planned Won't fix, can't repro, duplicate, stale Oct 29, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: commonjs @rollup/plugin-commonjs issue has workaround inconsistency Inconsistency between dev & build
Projects
None yet
Development

No branches or pull requests

4 participants