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

feat: add importedCss and importedAssets properties to RenderedChunk type #6629

Merged
merged 6 commits into from
Mar 3, 2022

Conversation

aleclarson
Copy link
Member

@aleclarson aleclarson commented Jan 25, 2022

Description

Replace the internal chunkToEmittedCssFileMap and chunkToEmittedAssetsMap variables with public properties added by Vite to RenderedChunk objects in the renderChunk phase.

These can be useful for Vite-based frameworks that generate their own HTML. We also want to move toward decoupling the internal HTML/CSS/Asset plugins, and this feature helps with that.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-dev patak-dev added the p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) label Jan 27, 2022
aleclarson added a commit to alloc/saus that referenced this pull request Jan 27, 2022
aleclarson added a commit to alloc/saus that referenced this pull request Jan 28, 2022
bluwy
bluwy previously approved these changes Feb 1, 2022
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a clean refactor to me.

@patak-dev
Copy link
Member

Queued for discussion in the next team meeting 👍🏼

benmccann
benmccann previously approved these changes Feb 7, 2022
@Niputi Niputi added this to the 2.9 milestone Feb 11, 2022
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aleclarson we talked about the PR in yesterday's meeting and we are good to move forward. The only change to merge is to nest importedAssets and importedCss in chunk.viteMetadata, so we can later continue to expand the metadata without possible collisions with rollup (or other bundlers like WMR).

We also considered using an internal WeakMap and exposing a getViteChunkMetadata(chunk) function instead but it looks less ergonomic to use.

@ElMassimo
Copy link
Contributor

I can make the suggested changes if needed (if you would prefer @aleclarson).

@aleclarson
Copy link
Member Author

@ElMassimo No need to ask, go right ahead ;)

@ElMassimo
Copy link
Contributor

@patak-dev @aleclarson Applied the suggested changes in a pull request targeting this branch.

Please review, and if it looks good we can go ahead and merge that one first.

Although it's more verbose, it makes it easier to avoid collisions with
rollup or other bundlers like WMR.
@aleclarson aleclarson dismissed stale reviews from benmccann and bluwy via c23c5de February 22, 2022 18:01
Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks @ElMassimo for doing the changes. Lets check if we can start the 2.9 beta next week and merge this one then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants