Skip to content

Conversation

@agilgur5
Copy link
Owner

Summary

Use rollup-plugin-node-externals instead of custom external code

Details

@agilgur5 agilgur5 added the kind: internal Changes only affect the internals and not the API or usage label Jun 11, 2022
@agilgur5 agilgur5 force-pushed the deps-rollup-plugin-node-externals branch from 63fb3f7 to 0b00c7e Compare June 11, 2022 02:55
@agilgur5
Copy link
Owner Author

Travis CI just didn't run for whatever reason so I finally bit the bullet and migrated to GH Actions in #84. Rebased on top of that now

@codecov
Copy link

codecov bot commented Jun 11, 2022

Codecov Report

Merging #83 (6ed1a63) into main (6ed1a63) will not change coverage.
The diff coverage is n/a.

❗ Current head 6ed1a63 differs from pull request most recent head ce2411a. Consider uploading reports for the commit ce2411a to get more accurate results

@@            Coverage Diff            @@
##              main       #83   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           75        75           
  Branches         9         9           
=========================================
  Hits            75        75           

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 6ed1a63...ce2411a. Read the comment docs.

- instead of the custom JS code I wrote
  - I was thinking of separating that into a plugin, but someone already
    did that!
    - and, importantly, also covered submodules, unlike _most_ of the
      externals plugins
      - see regex here: https://github.com/Septh/rollup-plugin-node-externals/blob/11a7b4454f57c76436e71ecead0cc59ab0cc3b80/src/index.ts#L106

- put it _before_ `node-resolve` as the docs state

- reduces my code a bit, which is always nice, and will make it easier
  to split my Rollup config into its own "preset" package later on
@agilgur5 agilgur5 force-pushed the deps-rollup-plugin-node-externals branch from 0b00c7e to ce2411a Compare June 11, 2022 03:00
@agilgur5 agilgur5 added the scope: dependencies Pull requests that update a dependency file label Jun 11, 2022
Copy link
Owner Author

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM, yay for simpler code!

I did also confirm locally that this produces an equivalent bundle ✅

@agilgur5 agilgur5 merged commit 0154531 into main Jun 11, 2022
@agilgur5 agilgur5 deleted the deps-rollup-plugin-node-externals branch June 11, 2022 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: internal Changes only affect the internals and not the API or usage scope: dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants