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

The world is not ready for esm yet #26

Merged
merged 9 commits into from
Oct 4, 2021
Merged

Conversation

alexeyraspopov
Copy link
Owner

@alexeyraspopov alexeyraspopov commented Oct 1, 2021

Rationale

With this PR we break any possible compatibility with nanocolors and colorette for the sake of larger ecosystem support, smaller package size, and the ability to enforce color (via custom API).

  • Supporting ESM-first is just too much pain (Fix React Native support #25, Add CJS version #2). Maybe the story will change in a year or two
  • Tree-shaking is not beneficial for a library which primary use is Node.js CLI tooling
  • Import of an API object is more flexible than adding every single formatting function to the import

Additionaly, I decided to make another change here, while I'm at it: using tabs instead of spaces. Because I like tabs with size 2 and it also reduces package size. The PR includes .editorconfig so everyone who's using the plugin should be fine. Also, GitHub properly supports it on the website.

Preliminary checking the package size (via clean-publish and npm publish --dry-run) shows following stats:

package size:  2.4 kB
unpacked size: 5.6 kB

That's a win-win

Closes #4
Closes #13
Closes #22
Closes #25

picocolors.d.ts Outdated
bgMagenta: (input: string) => string,
bgCyan: (input: string) => string,
bgWhite: (input: string) => string,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Need createColors type too.

@alexeyraspopov
Copy link
Owner Author

@ai, I made another attempt to do .d.ts but I need someone to verify it since I'm not quite good at it

@ai
Copy link
Contributor

ai commented Oct 2, 2021

@alexeyraspopov .d.ts looks good, but createColors’s result doesn’t have createColors. It is better to use:

export type Formatter = (input: string | number | null | undefined) => string;

export interface Colors {
	isColorSupported: boolean;
	reset: Formatter;
	bold: Formatter;
	dim: Formatter;
	italic: Formatter;
	underline: Formatter;
	inverse: Formatter;
	hidden: Formatter;
	strikethrough: Formatter;
	black: Formatter;
	red: Formatter;
	green: Formatter;
	yellow: Formatter;
	blue: Formatter;
	magenta: Formatter;
	cyan: Formatter;
	white: Formatter;
	gray: Formatter;
	bgBlack: Formatter;
	bgRed: Formatter;
	bgGreen: Formatter;
	bgYellow: Formatter;
	bgBlue: Formatter;
	bgMagenta: Formatter;
	bgCyan: Formatter;
	bgWhite: Formatter;
}

export = Colors & { createColors: (enabled: boolean) => Colors };

Also, it is useful for some TS projects to be able to re--use Formatter and Colors types.

@alexeyraspopov alexeyraspopov merged commit 2bbb880 into main Oct 4, 2021
@alexeyraspopov alexeyraspopov deleted the feature/ForceCJS branch October 4, 2021 04:19
@@ -5,20 +5,19 @@
A tinier and faster alternative to [nanocolors](https:/ai/nanocolors). Andrey, are you even trying?

```javascript
import { green, italic } from "picocolors";
import pc from "picocolors";
Copy link

Choose a reason for hiding this comment

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

Explicit commonjs exports allows to use named imports in node

exports.red = red;
// usage
import { red } from 'picocolors';

wopian added a commit to wopian/kitsu-season-trends that referenced this pull request Oct 8, 2021
0.2.0 broke ESM compatibility and no longer works in this project. See alexeyraspopov/picocolors#26 for the odd reasoning for this change.

Maybe consider moving to another small colour library that isn't stuck in the ancient land of CJS
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.

Add default export to CJS Add API to manually change color mode
3 participants