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

Updates for Node 14 and ESM #30

Merged
merged 25 commits into from
Jun 16, 2020
Merged

Updates for Node 14 and ESM #30

merged 25 commits into from
Jun 16, 2020

Conversation

kristoferbaxter
Copy link
Contributor

@kristoferbaxter kristoferbaxter commented Apr 28, 2020

  • ESM format source
  • Autogenerate CJS main field source
  • Use tap with parallelized tests
  • Leverage volta for pinning node and yarn version
  • Output export maps for node 14 and above
  • Test node versions 6.x, 8.x, 10.x, 12.x, 14.x

NOTE This PR isn't meant to be consumed as is, but as a heads up of how some of this might work.

This PR is now ready for review, maintaining compatibility with older node versions but using ESM when available.


Closes #31

@kristoferbaxter kristoferbaxter marked this pull request as draft April 28, 2020 18:41
index.mjs Outdated Show resolved Hide resolved
@kristoferbaxter kristoferbaxter marked this pull request as ready for review April 28, 2020 20:58
@kristoferbaxter
Copy link
Contributor Author

Bench results with Node 14.

# All Colors
  ansi-colors x 194,188 ops/sec ±0.33% (95 runs sampled)
  chalk x 635,888 ops/sec ±0.30% (95 runs sampled)
  kleur x 744,164 ops/sec ±0.50% (95 runs sampled)

# Stacked colors
  ansi-colors x 25,717 ops/sec ±0.38% (93 runs sampled)
  chalk x 604,484 ops/sec ±0.30% (97 runs sampled)
  kleur x 100,552 ops/sec ±0.25% (93 runs sampled)

# Nested colors
  ansi-colors x 77,884 ops/sec ±0.20% (94 runs sampled)
  chalk x 147,621 ops/sec ±0.61% (97 runs sampled)
  kleur x 155,266 ops/sec ±1.80% (94 runs sampled)

Copy link
Owner

@lukeed lukeed 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 for this. Hope you don't mind that I reduced some of the moving pieces. The ideas and the approaches are the same 👍

However, do you mind removing the test changes? I'm not opposed to the new tests (I'd have some suggestions for them), but they're not working in CI – and even if they were, it's too many large-scale moving chunks at once. I'd feel much better merging an exports PR that proves existing usage is preserved (eg, everything via require; to start...).

I'd much rather look at & discuss test changes separately.

Hope you understand!

readme.md Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@lukeed
Copy link
Owner

lukeed commented Apr 29, 2020

Also, the tests may be parallelized now, but they're significantly slower.

Node 6 tape: 24ms
Node 10 tape: 13ms
Node 14 tape: 12ms

Node 14 tap w/ require (new): 781ms
Node 14 tap w/ import (new): 702ms

@kristoferbaxter
Copy link
Contributor Author

Happy to disentangle. Will be able to look at this in a few days.

- typescript can infer from this type correctly for ESM, CJS, and TS. Using `export default` produces type errors in CJS files
Copy link
Owner

@lukeed lukeed 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 @kristoferbaxter :)

I'm going to make some additional changes that bring back your dual/native testing, but I don't want to include that noise here.

Appreciate your patience

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.

Make it available in pika cdn
2 participants