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

fix: move @kurkle/color to dependencies #10917

Merged
merged 1 commit into from Nov 30, 2022
Merged

fix: move @kurkle/color to dependencies #10917

merged 1 commit into from Nov 30, 2022

Conversation

dangreen
Copy link
Collaborator

fixes #10915

@dangreen dangreen marked this pull request as ready for review November 29, 2022 13:49
@etimberg etimberg merged commit 8283ad5 into chartjs:master Nov 30, 2022
@dangreen dangreen deleted the fix-10915 branch December 3, 2022 10:01
@theengineear
Copy link

First off — thanks much for developing / maintaining this library. I started with version "3" and have so far found it very easy to work with!

Ok… on to my ask!

@etimberg / @kurkle / @LeeLenaleee — This seems like a big change to go from no dependencies to some dependencies. I initially picked this library because (1) I could read through the code I would be using in my application reasonably easily, (2) I didn't have to link out to other code in the dependencies block to further understand the implications of including this library and (3) it was network-importable (i.e., I could import this in the browser without a build step).

As shown in the npm image, that import '@kurkle/color'; line cannot be understood by a browser without import maps (which aren't yet supported in all modern browsers). Am I doing something wrong or misunderstanding the expected import mechanism in the newest version (version "4")?

Screenshot 2022-12-16 at 7 17 02 PM

Screenshot 2022-12-16 at 7 20 31 PM

@dangreen
Copy link
Collaborator Author

@theengineear Hi. To use chart.js directly in a browser you should use umd bundle: https://unpkg.com/chart.js@4.1.0/dist/chart.umd.js . Or you can use Skypack to import it as module: https://cdn.skypack.dev/chart.js

@theengineear
Copy link

For security reasons, we install our dependencies into a /vendor folder and import them from there. Additionally, for static / offline development, we keep the files installed locally.

We can probably just stay on 3.0 at least until import maps are supported. Do y'all plan to add more dependencies? Just curious about the library's intended direction.

Also, I'll make another plug for having a zero-dependency library — it was a major reason my team went with Chart.js. It made it super easy to work with. Perhaps there's a way to register @kurkle/color at boot-time so that this isn't a strict requirement in the dependency tree?

I'm happy to open up an issue detailing some these concerns if that's helpful, by the way. I just figured I'd ping in the PR first since there's already context here 👌

@dangreen
Copy link
Collaborator Author

@theengineear You can download https://unpkg.com/chart.js@4.1.0/dist/chart.umd.js to your vendor directory

@theengineear
Copy link

Ok, I'll try adding that to my package.json#dependencies 👌

@JamesMilneFilmlight
Copy link

Just thought I'd add my 2 cents that this change also breaks my ability to vendor Chart.js into my source tree as an ESModule. I wanted to be able to use Chart.js via import Chart from "/path/to/chart.js". but the dist/chart.js file generated by rollup still includes the import '@kurkle/color'; directive.

This means I can't use dist/chart.js without further processing.

@dangreen
Copy link
Collaborator Author

dangreen commented Jan 17, 2023

@JamesMilneFilmlight Hi. You can use UMD bundle

import Chart from "./path/to/chart.js/dist/chart.umd.js"

@JamesMilneFilmlight
Copy link

Hi @dangreen, OK yes I can work with this for now.

@theengineear
Copy link

FYI for anyone that finds this. import maps have pretty good support at this point and you can add this in your document (in the head, before any other type="module" scripts are loaded):

    <script type="importmap">
      {
        "imports": {
          "@kurkle/color": "/vendor/@kurkle/color/dist/color.esm.js"
        }
      }
    </script>

For us, we can now add it to the root html document, but because all our demo pages are stand-alone, it also means we need to add it to each demo page which imports a component which ultimately imports chart.js (explicitly or implicitly).

@klebba
Copy link

klebba commented Apr 1, 2024

Thanks @theengineear — the import map is a good workaround and follows web standards.

@dangreen A UMD bundle is not really a replacement for a native JS module. It would be great if Chart.js went back to zero dependencies, would you be open to a PR to eliminate the need for @kurkle/color?

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.

Cannot find module '@kurkle/color' or its corresponding type declarations.
7 participants