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(commonjs): Only proxy detected commonjs entry points #1180

Merged
merged 2 commits into from
Jun 24, 2022

Conversation

lukastaegert
Copy link
Member

Rollup Plugin Name: commonjs

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:
Resolves #1169

Description

This will ease the issue in #1169 by only proxying detected CommonJS entry points (and not just every entry point). It does so via:

  • Initially using the include/exclude filtering mechanism
  • Using the detected module type information

Thus if you do not have commonjs entry points, you should no longer see the ?entry-proxy modules replacing your entry points.

@silkfire
Copy link

silkfire commented Apr 30, 2022

NB: Node.js v12 went EOL today, maybe remove it and introduce v17 as it goes LTS tomorrow?

image

@lukastaegert lukastaegert force-pushed the commonjs/reduce-entry-proxying branch from 92d8600 to 97b2f2e Compare May 4, 2022 04:05
haoqunjiang added a commit to haoqunjiang/vite that referenced this pull request Jun 23, 2022
Well, the built bundle doesn't work yet.
Blocked by rollup/plugins#1180
@haoqunjiang
Copy link
Contributor

haoqunjiang commented Jun 24, 2022

I can confirm that this PR fixes most issues that prevent Vite from updating the commonjs plugin.

Before patching with this fix, most test cases in Vite fail (likely due to this bug interfering with the default .html entry); after patching, only 1 case is still failing (and I'm still investigating, maybe not caused by the CJS plugin).

@lukastaegert
Copy link
Member Author

I will ensure it is released

@lukastaegert lukastaegert merged commit 6ba7148 into master Jun 24, 2022
@lukastaegert lukastaegert deleted the commonjs/reduce-entry-proxying branch June 24, 2022 10:50
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.

CommonJS plugin breaks isEntry for entry points
4 participants