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: conditional exports #419

Merged
merged 4 commits into from Feb 14, 2022
Merged

feat: conditional exports #419

merged 4 commits into from Feb 14, 2022

Conversation

ricokahler
Copy link
Owner

@ricokahler ricokahler commented Feb 1, 2022

This PR:

  1. Adds conditional exports to color2k (see below)
  2. Includes allow custom background color #405
  3. Cleans up some ESLint package deps

Color2k handles its own bundle with the following entry points and targets:

  • mainnode 4
  • unpkg — browserslist defaults
  • modulenode 6 for ES2015 support defined by rollup a while ago
  • exports.importnode 12 for ES2019 support (esbuild and node >12 in ES mode uses this entry point)
  • exports.defaultnode 12 for CJS node 12 support, since export maps weren't support until node 12
  • exports.package.json — for backwards compat to importing package.json (since it's restricted to import other files now)

The following consumers have been manually tested:

  • legacy node (e.g. node 4)
  • <script type="module"> via skypack
  • deno import via skypack
  • jest with the default babel setup — this uses jest-resolve which uses resolve (maintained by none other than @ljharb! 😄). This works but uses the legacy main CJS bundle due to the complexity of closing Support ESM resolution browserify/resolve#222. Note from the jest folks, they are highly unlikely to switch from resolve.
  • @babel/register with [@babel/preset-env, {targets: 'node 12'}] — uses the CJS exports.require entry point
  • @rollup/plugin-node-resolve — uses the exports.import entry point
  • webpack 5 — uses enhanced-resolve with the conditions ["require", "module"] for CJS and ["import", "module"] for ESM. resolves to the exports.import entry point
  • webpack 3 — this works and uses the legacy module entry point
  • node LTS with both ESM/CJS — works as expected

Closes #413

@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #419 (d1c87a7) into main (37af5b7) will increase coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #419      +/-   ##
==========================================
+ Coverage   96.44%   96.46%   +0.01%     
==========================================
  Files          23       23              
  Lines         197      198       +1     
  Branches       44       45       +1     
==========================================
+ Hits          190      191       +1     
  Misses          7        7              
Impacted Files Coverage Δ
src/hasBadContrast.ts 80.00% <66.66%> (+5.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 37af5b7...d1c87a7. Read the comment docs.

@ricokahler ricokahler linked an issue Feb 1, 2022 that may be closed by this pull request
Copy link
Contributor

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This works in theory :-) i'd have to look at the build output to be sure.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

seems good; short of an actual node 4 test

@ricokahler
Copy link
Owner Author

@ljharb alright i've spent waaaay to long reading up on these exports and manually testing but I think we're finally good to go after a node 4 test. I'll merge after getting that done.

Thanks for your feedback!

@ljharb
Copy link
Contributor

ljharb commented Feb 2, 2022

I will point out that adding "exports" is almost always a breaking change, so you may want to do this as a semver-major bump.

@ricokahler ricokahler force-pushed the feat/conditional-exports branch 2 times, most recently from 4a5cb52 to 76d9139 Compare February 14, 2022 02:15
Co-authored-by: ljharb <ljharb@gmail.com>

BREAKING CHANGE: This package now includes [conditional package exports][docs]. Most consumers should be able to upgrade without any issues, however, package exports limits the allowed entry points inside of Node.js and also affects consumption in other bundlers.

- `main` targets been updated to support down to node 4 (ES5)
- `module` targets has update to support down to node 6 (ES2015)
- `exports` targets have been set to node 12 (ES2019)
- `unpkg` targets have been set to browserslist `defaults`

[docs]: https://nodejs.org/api/packages.html#conditional-exports
Co-authored-by: Novolokov <83767977+Novolokov@users.noreply.github.com>

Squashed commit of the following:

commit 32afa52
Merge: d0b4620 37af5b7
Author: Rico Kahler <ricokahler@gmail.com>
Date:   Sun Feb 13 19:41:05 2022 -0500

    Merge branch 'main' into patch-1

commit d0b4620
Author: Novolokov <83767977+Novolokov@users.noreply.github.com>
Date:   Sat Dec 4 13:04:24 2021 +0100

    allow custom background color
@ricokahler ricokahler merged commit 5bc4720 into main Feb 14, 2022
@ricokahler ricokahler deleted the feat/conditional-exports branch February 14, 2022 02:55
@github-actions
Copy link

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

node 4 compatibility Bundled outputs
2 participants