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(legacy): fix conflict with the modern build on css emitting #6584

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

sodatea
Copy link
Member

@sodatea sodatea commented Jan 21, 2022

Description

Fixes #3296
Fixes #5325
Supersedes #3317

The asset emitting conflict may also exist for other types of assets,
but let's fix the CSS one first.

The conflict here is due to the hasEmitted flag that was originally
intended to avoid duplicated CSS for multiple output formats
6bce108#diff-2cfbd4f4d8c32727cd8e1a561cffbde0b384a3ce0789340440e144f9d64c10f6R262-R263

When the legacy plugin is used, the flag was set to true for the
emitted CSS of the legacy bundle.
But the legacy plugin would remove all its emitted assets later to avoid
duplication.

if (isLegacyBundle(bundle, opts)) {
// avoid emitting duplicate assets
for (const name in bundle) {
if (bundle[name].type === 'asset') {
delete bundle[name]
}
}

So this logic results in no CSS to be actually emitted.

In this PR, I used a __vite_skip_asset_emit__ flag to prevent the
CSS generateBundle from executing for the legacy build.
If other asset emitting plugins encounter similar issues, this flag
can be reused.

Additional context

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

Fixes vitejs#3296
Supersedes vitejs#3317

The asset emitting conflict may also exist for other types of assets,
but let's fix the CSS one first.

The conflict here is due to the `hasEmitted` flag that was originally
intended to avoid duplicated CSS for multiple output formats
vitejs@6bce108#diff-2cfbd4f4d8c32727cd8e1a561cffbde0b384a3ce0789340440e144f9d64c10f6R262-R263

When the legacy plugin is used, the flag was set to `true` for the
emitted CSS of the legacy bundle.
But the legacy plugin would remove all its emitted assets later to avoid
duplication.
So this logic results in no CSS to be actually emitted.

In this PR, I used a `__vite_skip_asset_emit__` flag to prevent the
CSS `generateBundle` from executing for the legacy build.
If other asset emitting plugins encounter similar issues, this flag
can be reused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vite doesn't emit shared css for legacy build. vite 打包之后 css 文件丢失
3 participants