Skip to content

ESM-only#4542

Open
florian-lefebvre wants to merge 16 commits into
lovell:mainfrom
florian-lefebvre:feat/esm-only
Open

ESM-only#4542
florian-lefebvre wants to merge 16 commits into
lovell:mainfrom
florian-lefebvre:feat/esm-only

Conversation

@florian-lefebvre

@florian-lefebvre florian-lefebvre commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Closes #4539

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

Tests pass locally for me. @lovell can you run the workflows?

@florian-lefebvre florian-lefebvre marked this pull request as ready for review June 11, 2026 14:05
@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

@lovell I think I fixed everything (coverage and node 20), should be good to go

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

I just tried running the failing tests locally on Windows 11 and can't replicate, so I think that they should pass now that lint is fixed, but no guarantee :/

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

I think the issue was around the font file so I tried another approach of having the font file directly in the repo. I tried it locally on windows 11 and it worked but again, no guarantee. If that doesn't work, I'm out of ideas

@florian-lefebvre

Copy link
Copy Markdown
Contributor Author

It worked 🎉

Comment thread lib/colour.js
@@ -4,7 +4,7 @@
*/

import color from '@img/colour';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@img/colour is essentially the CommonJS version of the color package:

This package converts the color package and its dependencies, all of which are MIT-licensed, to CommonJS.

Since sharp is now ESM-only, this seems like a good time to migrate to color directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll let Lovell decide but I think it makes sense

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For context I also opened e18e/module-replacements#786 about it

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.

ESM-only

2 participants