-
Notifications
You must be signed in to change notification settings - Fork 45
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
Support named exports in the ESM mode #60
base: main
Are you sure you want to change the base?
Conversation
256652f
to
df4e2db
Compare
Allow importing single exported named functions: import { bold, green } from "picocolors" Expose an ESM module with named exports: `picocolors.mjs`. Generate its content from `picololors.js` by `build.mjs`. Test that the exports can be imported by `test/esm.mjs`. Also upgrade development dependencies. Keep the same major versions of `chalk` and `clean-publish` not to break the legacy Node.js tests. Also include tests with Node.js 20 and move versions older than 18 to the legacy group. Upgrade GitHub actions for modern Node.js.
Export single named exports from `picocolors.d.ts`. Test the functionality by `test/types.mts`. Include both `*.ts` files in the package. Include the test in the GitHub workflow.
df4e2db
to
80f5db4
Compare
"sideEffects": false, | ||
"description": "The tiniest and the fastest library for terminal output formatting with ANSI colors", | ||
"scripts": { | ||
"build": "node build-esm.mjs && tsc --module esnext tests/types.mts", | ||
"test": "node tests/test.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be more logical to put the compiling of the types tests during the test script? As it is a way to tests that types are correct.
Also maybe adding --noEmit
option to not produce any compiled files of the test, no ?
These are very good changes, but we're not yet dropping these much of old LTS Node.js versions. On the subject, I honestly can hardly understand the reason you'd want to import every single color separately. It just puzzles me how this can be convenient. With default export I don't need to go back and forth updating import (even if auto import enabled in an editor of choice). This feels like an unnecessary mental overhead. |
Pressing the |
This is Node.js, there is no tree-shaking, both |
Proposal
Allow importing single exported named functions:
Problem
If named exports are used today, the following error will be thrown:
Changes
Expose an ESM module with named exports:
picocolors.mjs
. Generate its content frompicololors.js
bybuild.mjs
. Test that the exports can be imported bytest/esm.mjs
.Also upgrade development dependencies. Keep the same major versions of
chalk
andclean-publish
not to break the legacy Node.js tests.Also include tests with Node.js 20 and move versions older than 18 to the legacy group. Upgrade GitHub actions for modern Node.js.
Also fixes #50.
Also fixes #59.