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 preliminaryFileName to OutputChunk #5086

Merged
merged 4 commits into from
Aug 9, 2023
Merged

feat: add preliminaryFileName to OutputChunk #5086

merged 4 commits into from
Aug 9, 2023

Conversation

lsdsjy
Copy link
Contributor

@lsdsjy lsdsjy commented Aug 4, 2023

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

List any relevant issue numbers:
resolves #5080

Description

@vercel
Copy link

vercel bot commented Aug 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rollup ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 9, 2023 5:17am

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Thank you very much! Unfortunately, there are a few more things that need to be done for this feature to be release:

  • The new property needs to be added to types.d.ts. Most tests fail for TypeScript errors because of this
  • There is a lint issue because properties in object literals are sorted
  • There should be a test asserting the value of the new property. It could be that this would already be covered in tests and they just need some adjustments. You would see when fixing the TypeScript issue and running the tests.
  • The new property needs to be documented in various places. At least here and here

@lsdsjy
Copy link
Contributor Author

lsdsjy commented Aug 8, 2023

@lukastaegert Hi, thanks for the comment! I've pushed some more commits. However the checks seem to be not running and I don't know why...

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #5086 (06c34ce) into master (6787f44) will decrease coverage by 0.01%.
Report is 7 commits behind head on master.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master    #5086      +/-   ##
==========================================
- Coverage   98.97%   98.96%   -0.01%     
==========================================
  Files         226      226              
  Lines        8362     8391      +29     
  Branches     2294     2303       +9     
==========================================
+ Hits         8276     8304      +28     
  Misses         31       31              
- Partials       55       56       +1     
Files Changed Coverage Δ
src/utils/FileEmitter.ts 100.00% <ø> (ø)
src/utils/decodedSourcemap.ts 93.54% <92.59%> (+7.83%) ⬆️
src/Chunk.ts 99.65% <100.00%> (+<0.01%) ⬆️
src/Module.ts 100.00% <100.00%> (ø)
src/utils/collapseSourcemaps.ts 90.62% <100.00%> (+0.19%) ⬆️
src/utils/getOriginalLocation.ts 100.00% <100.00%> (ø)

@lukastaegert lukastaegert merged commit e3b614c into rollup:master Aug 9, 2023
10 of 12 checks passed
@rollup-bot
Copy link
Collaborator

This PR has been released as part of rollup@3.28.0. You can test it via npm install rollup.

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.

A way to get finalized file name from non-finalized file name in generateBundle hook
3 participants