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

fix(bundler-webpack): add conditionNames to import correct file #1475

Closed
wants to merge 4 commits into from
Closed

fix(bundler-webpack): add conditionNames to import correct file #1475

wants to merge 4 commits into from

Conversation

Mister-Hope
Copy link
Member

@Mister-Hope Mister-Hope commented Jan 24, 2024

See https://webpack.js.org/configuration/resolve/#resolveconditionnames

This solves a lot of issues.

I previously use alias with conditional checks (app.env.isDev, app.env.isBuild and bundler names) to ensure the correct file is used, but this can obviously fixed upstream here.

For example, when importing @vue/repl with webpack, @vue/repl import vue/component-sfc in it. And the latter use a condition export like this:

  "exports": {
    ".": {
      "types": "./dist/compiler-sfc.d.ts",
      "node": "./dist/compiler-sfc.cjs.js",
      "module": "./dist/compiler-sfc.esm-browser.js",
      "import": "./dist/compiler-sfc.esm-browser.js",
      "require": "./dist/compiler-sfc.cjs.js"
    },
  },

Then webpack should pack ./dist/compiler-sfc.esm-browser.js for it, and use "./dist/compiler-sfc.cjs.js" for SSR.

Another case can be bcrypt-ts

  "exports": {
    ".": {
      "types": "./dist/node.d.mts",
      "browser": "./dist/browser.mjs",
      "node": "./dist/node.mjs",
      "import": "./dist/node.mjs",
      "require": "./dist/node.cjs",
      "default": "./dist/node.cjs"
    },
  },

The crypto module is different under node ('node:crypto') and browser (window.Crypto)

@Mister-Hope Mister-Hope changed the title fix(bundler-webpack): add conditionNames to ensure importing correct … fix(bundler-webpack): add conditionNames to import correct file Jan 24, 2024
@meteorlxy
Copy link
Member

@Mister-Hope Any minimal reproduction for this issue? I could not reproduce it when trying to add e2e cases

@Mister-Hope
Copy link
Member Author

@meteorlxy
Copy link
Member

@Mister-Hope Could help to create a valid reproduce in imports/conditional-exports e2e test?

@coveralls
Copy link

coveralls commented Jan 25, 2024

Pull Request Test Coverage Report for Build 7649631525

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 40.764%

Totals Coverage Status
Change from base Build 7649311635: -0.02%
Covered Lines: 676
Relevant Lines: 1701

💛 - Coveralls

@Mister-Hope Mister-Hope closed this by deleting the head repository Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants