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

Updated fork #321

Open
regorxxx opened this issue Nov 8, 2023 · 16 comments
Open

Updated fork #321

regorxxx opened this issue Nov 8, 2023 · 16 comments

Comments

@regorxxx
Copy link

regorxxx commented Nov 8, 2023

An updated fork can be found here, with all dependencies updated, vulnerabilities fixed and new features (along relevant pendant pull request).

https://github.com/regorxxx/chroma.js

Docs have also been updated. Can be replaced directly without changes.

@trusktr
Copy link

trusktr commented Nov 29, 2023

@gka @regorxxx @iyinchao @stormwarning @salvoravida @Artoria2e5 @OlaoluwaM @tstaylor7 @tremby @taisukef

Hello you all! I tagged you because you're either the original owner of this repo, or someone who most-recently made significant features and updates in a fork of this repo (I looked at the network graph).

I figured, so as not to let this project get stale, perhaps we can all get together and help maintain a fork of it.

I started a fork from @regorxxx's in a new org, and I've invited you all to the org (anyone else want to join, please comment here!):

https://github.com/chroma-js/chroma.js

I will taking some inspiration from @taisukef's fork and starting from @regorxxx's fork making it all work as vanilla ES Modules, and deleting all traces of CommonJS (we can optionally add a CommonJS build if people really need that, but CommonJS is the past, ES Modules are the present and the future).

@tremby
Copy link

tremby commented Nov 29, 2023

Thanks for pointing it out. If I reach for Chroma again at some point I'll consider using one fork or the other. But in truth I haven't felt much need for Chroma since browsers improved their colour space support to include things like Oklab and Oklch.

@tremby
Copy link

tremby commented Nov 29, 2023

Oh I see you invited me to join the org too. I'm not going to at this point; one reason is that I feel a bit uncomfortable about it being named "chroma-js" without signoff (as far as I know) from the original author.

@taisukef
Copy link

Thank you for pointing it out!
I use ES Modules on browsers and Deno, so I forked and converted CommonJS sources.
https://github.com/code4fukui/chroma-es/tree/es/src
I hope you will make good use of this repository.
Please let us know if there's anything missing or if you have any suggestions.

@trusktr
Copy link

trusktr commented Nov 30, 2023

Oh I see you invited me to join the org too. I'm not going to at this point; one reason is that I feel a bit uncomfortable about it being named "chroma-js" without signoff (as far as I know) from the original author.

That's easy to change. The original author @gka is also invited.

GitHub does not allow user names or org names with a period, so github.com/chroma.js is not allowed, and github.com/chromajs and github.com/chroma are both already taken by unrelated projects. So github.com/chroma-js was the only close-enough option.

@trusktr
Copy link

trusktr commented Nov 30, 2023

Alright folks, the great work from @regorxxx and @taisukef is merged. The code is now all ES Modules:

https://github.com/chroma-js/chroma.js

Unlike your fork @taisukef, this keeps package.json and scripts for building and publishing for Node.js. But otherwise, most of the ESM conversions merged cleanly, and I only had to additionally convert @regorxxx's new and files. I think this should also work in Deno. Let me know if you can import from here for example:

https://raw.githack.com/chroma-js/chroma.js/main/index.js

When I run the docs, the colorful sidebar is missing (even on your fork @regorxxx):

Screenshot 2023-11-29 at 4 41 31 PM

Need to figure out where that went.

All scripts except test are working. The tests need to be updated to a modern test runner because the current test files are CommonJS and they cannot import ES Modules. I'll update the test runner next...

The dev process does not work in Windows at all. That is going to be fixed...

The root /chroma.js (and all other build outputs) are now ES Modules, and the changelog now shows that import is required for importing:

  • BREAKING: conversion of code base to ES Modules. All build outputs are now ES Modules. The global var script is no longer available, use import to import chroma.js. For example:
    <script type="module">
      import chroma from 'https://raw.githack.com/chroma-js/chroma.js/main/index.js'
      // ... use chroma as before ...
    </script>

@trusktr
Copy link

trusktr commented Nov 30, 2023

I force pushed to https://github.com/chroma-js/chroma.js as there was one small mistake in the merge with an import path, now the test/html/*.html files all work as expected. @taisukef curious to see if this works for you in Deno now (it would have failed until this force push).

@trusktr
Copy link

trusktr commented Nov 30, 2023

Aaaaaaalrighty! An initial working test for Deno is pushed @taisukef.

    "test:deno": "deno run test/test-deno.js",

@trusktr
Copy link

trusktr commented Nov 30, 2023

Most tests passing in Chrome browser running as vanilla ES Modules. Need to iron a few out:

Chrome: |██████████████████████████████| 50/50 test files | 1956 passed, 69 failed

Finished running tests, watching for file changes...

@trusktr
Copy link

trusktr commented Nov 30, 2023

@regorxxx @taisukef there's one particular test where the numbers are quite different between both of your forks. I'm wondering why that is. Here are the tests in question:

https://github.com/chroma-js/chroma.js/blob/340438a65d16e594e587f7fe72b98b0f924294d6/test/bezier.test.js#L133-L238

Both of you forked from this repo's 2.4.2 release, and one fork changed something that modified the numbers and I'm wondering what's correct.

@regorxxx
Copy link
Author

regorxxx commented Dec 9, 2023

@trusktr The original repo had wrong values and wrong tests.

Also see this:
#228

I also added proper n-degree bezier interpolation.

All my changes are documented on the changelog ;)

@regorxxx
Copy link
Author

regorxxx commented Dec 9, 2023

Once I have time, if I find working with ES modules is easier for me, I will help with that.

Right now I'm using chroma within a context where modules are not available and only just plain JS loading of files or require. If I can not work around that, will continue developing my fork.

@trusktr
Copy link

trusktr commented Dec 10, 2023

The original repo had wrong values and wrong tests.
Also see this: #228
documented on the changelog ;)

@regorxxx ah, thanks!

if I find working with ES modules is easier for me, I will help with that.

I hope you will!

ESM is the future-forward module standard for all JS runtimes, so we should support it as a default anyway.

We can add CommonJS or global build back fairly easily, but:

context where modules are not available and only just plain JS loading of files or require

Out of curiousity, what context is that and how does it stop you from using ESM? Is it an old browser or Node version? All browser and non-browser JS runtimes today support import(...) in any script (module or not).

@regorxxx
Copy link
Author

regorxxx commented Dec 10, 2023

@trusktr Not a browser related context, but foobar2000. There are JS plugins for it, to create any frontend/backend element, so I can use most libraries with full support for ES6. But import/require, etc. are not available. I can polyfill require, but can not do that with import.

@regorxxx
Copy link
Author

regorxxx commented Dec 26, 2023

Added a few more features on 2.7.0, along a fix to the 'npm run docs-preview' command which was not working on some OSes.

Also a logo for the library
image

@trusktr
Copy link

trusktr commented Feb 26, 2024

@trusktr Not a browser related context, but foobar2000. There are JS plugins for it, to create any frontend/backend element, so I can use most libraries with full support for ES6. But import/require, etc. are not available.

Ah I see. We can remedy that. We can have a build step that procudes the global build (f.e. UMD format or similar). That way by default people can use ES modules but old environments can still have an option.

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

No branches or pull requests

4 participants