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

NPM missing source #6138

Closed
2 of 3 tasks
anton-shchyrov opened this issue Mar 15, 2019 · 5 comments
Closed
2 of 3 tasks

NPM missing source #6138

anton-shchyrov opened this issue Mar 15, 2019 · 5 comments

Comments

@anton-shchyrov
Copy link

Documentation Is:

  • Missing or needed
  • Confusing
  • Not Sure?

Please Explain in Detail...

In npm packet version 2.8.0-rc.1 and 2.8.0 missing source code. There is only a folder dist. Is this what was meant?

@simonbrunel
Copy link
Member

Yes, because src/**/* should be considered private (see #6105). Instead, users should import 'chart.js' or require('chart.js') which will use the dist/Chart.js UMD build.

Also, we switched to rollup and now import CSS in our code via a rollup plugin. So even if the src folder would still be in the npm package, it would break projects accessing the source files directly.

@mojoaxel
Copy link
Contributor

mojoaxel commented Apr 2, 2019

Sadly I'm a little late here but I just wanted to let you folks know, that I also was using the src folder.
I'm also using rollup.js to build my application and I only include the modules from chart.js that I actually use. Now with the src folder removed in v2.8.0 this is not possible anymore. I cannot just include the dist version because that would go beyond my size limits.

Now I would have to create a seperate custom chart.js library with only the modules I need and build my own custom release or include the chart.js sources in my project what i don't want to do. 😢

I'm sorry that I've missed the discussions in #6105 Is there a chance to bring back the src folder?

@simonbrunel
Copy link
Member

@mojoaxel I understand your point and I'm sorry to read that we broke your integration.

I don't think it's a good idea to re-inject our sources in the npm package because you shouldn't rely on it for many reasons. We should be able to rename/move/delete whatever file under /src, change the way we import/organize modules, add more rollup tricky build steps that would be complicated to integrate into your workflow (e.g. injecting the CSS - #6048), etc. without to worry about breaking existing projects that would target /src/**/* files. We don't want to maintain backward compatibility on the /src files and that's why it should not be in the npm package.

I would rather try to find a way to generate an ESM build that supports tree-shaking (#5179) so you could import exactly what you need.

@simonbrunel
Copy link
Member

Alternatively (but not ideal), you could install Chart.js from GitHub directly:

 npm install -S https://github.com/chartjs/Chart.js/tarball/v2.8.0
// package.json
{
  "dependencies": {
    "chart.js": "https://github.com/chartjs/Chart.js/tarball/v2.8.0"
  }
}

So you will get the '/src' folder and be able to use it at your own risk.

@mojoaxel
Copy link
Contributor

mojoaxel commented Apr 2, 2019

@simonbrunel Thanks for your quick answer ❤️

We don't want to maintain backward compatibility on the /src files

That's a good point! Of course I never expected that, but others may do so.

I would rather try to find a way to generate an ESM build that supports tree-shaking

👍 That, of course would be the ideal solution. I'll try to monitor #5179 and adopt accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants