Skip to content

feat: add resolvedBy field to ResolvedId#4789

Merged
lukastaegert merged 11 commits intorollup:masterfrom
TrickyPi:feat/add-resolve-by
Jan 12, 2023
Merged

feat: add resolvedBy field to ResolvedId#4789
lukastaegert merged 11 commits intorollup:masterfrom
TrickyPi:feat/add-resolve-by

Conversation

@TrickyPi
Copy link
Copy Markdown
Member

@TrickyPi TrickyPi commented Jan 5, 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 #4773

Description

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 5, 2023

Codecov Report

Merging #4789 (aef4368) into master (ade0ee3) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4789      +/-   ##
==========================================
- Coverage   99.03%   99.02%   -0.02%     
==========================================
  Files         217      217              
  Lines        7773     7780       +7     
  Branches     2158     2161       +3     
==========================================
+ Hits         7698     7704       +6     
  Misses         24       24              
- Partials       51       52       +1     
Impacted Files Coverage Δ
src/ModuleLoader.ts 99.57% <100.00%> (-0.43%) ⬇️
src/utils/PluginDriver.ts 100.00% <100.00%> (ø)
src/utils/resolveId.ts 95.23% <100.00%> (+1.90%) ⬆️
src/utils/resolveIdViaPlugins.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Copy Markdown
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! Besides the comments I added, it would be nice to also update the documentation. I think this.resolve would be a good place to document it, but it should also be mentioned in resolveId that you can return a resolved field and what the effect is (it changes the return value in this.resolve). Also, we need to update the types in the documentation in at least these two places (not sure if there is another place where the ResolvedId type is used). Thanks for your hard work!

@TrickyPi TrickyPi force-pushed the feat/add-resolve-by branch from cc1257f to 1e9d774 Compare January 9, 2023 13:59
@TrickyPi TrickyPi force-pushed the feat/add-resolve-by branch from 1e9d774 to 1ed9fb4 Compare January 9, 2023 14:01
@TrickyPi TrickyPi changed the title feat: add resolveBy field to ResolvedId feat: add resolvedBy field to ResolvedId Jan 10, 2023
@TrickyPi TrickyPi force-pushed the feat/add-resolve-by branch from 87cbc6e to 1744073 Compare January 10, 2023 13:50
@lukastaegert lukastaegert merged commit b26a37f into rollup:master Jan 12, 2023
@lukastaegert
Copy link
Copy Markdown
Member

This PR has been released as part of rollup@3.10.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.

RFC: Add resolvedBy information to resolved ids

2 participants