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

Expose rendered module code to generateBundle hook #4028

Merged
merged 4 commits into from Mar 29, 2021

Conversation

btd
Copy link
Contributor

@btd btd commented Mar 28, 2021

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:

Description

This fixes issues i created #3458. I would like to use this info for simple analysis and better code size estimations in visualizer plugin.

Let me know pls, if it could be done better.

@codecov
Copy link

codecov bot commented Mar 28, 2021

Codecov Report

Merging #4028 (0823619) into master (85a3724) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4028   +/-   ##
=======================================
  Coverage   97.47%   97.47%           
=======================================
  Files         192      192           
  Lines        6773     6775    +2     
  Branches     1984     1985    +1     
=======================================
+ Hits         6602     6604    +2     
  Misses         84       84           
  Partials       87       87           
Impacted Files Coverage Δ
src/Chunk.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85a3724...0823619. Read the comment docs.

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.

Looks good, I think we can include this as it is, especially with the lazy evaluation. The only things missing to me are doc updates: code should be added both to 02-javascript-api.md at line 58 and 05-plugin-development.md at line 339.
Also I think returning null instead of undefined for the tree-shaken case might fit better with other APIs, but this is just my feeling.

@github-actions
Copy link

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install btd/rollup#expose-rendered-module-code

Once a build has completed, you can load it into the REPL by inserting the CircleCI build number of the ci/circleci: node-v12-latest build into the link below (unfortunately, we cannot auto-update this comment for PRs from forks at this time):

https://rollupjs.org/repl/?circleci=<build_number>

@btd
Copy link
Contributor Author

btd commented Mar 29, 2021

@lukastaegert thanks. I added docs and changed type to be string | null

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.

Looks great, thanks!

@lukastaegert lukastaegert merged commit 443dc97 into rollup:master Mar 29, 2021
@btd
Copy link
Contributor Author

btd commented Mar 29, 2021

Thank you

@btd btd deleted the expose-rendered-module-code branch March 29, 2021 14:08
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.

None yet

2 participants