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

handle 'chart' and 'mousetrap' dependencies through package.json #316

Closed
wants to merge 1 commit into from

Conversation

browner12
Copy link
Contributor

rather than linking to the CDNs, we'll handle these dependencies with our package manager. there is a minimal hit in filesize, but this gives us the benefit of keeping external dependencies up to date through one mechanism.

I think there's potential to take advantage of these libraries on other pages as well. I know, personally, mousetrap is something I install in every project.

rather than linking to the CDNs, we'll handle these dependencies with our package manager. there is a *minimal* hit in filesize, but this gives us the benefit of keeping external dependencies up to date through one mechanism.

I think there's potential to take advantage of these libraries on other pages as well.  I know, personally, mousetrap is something I install in every project.
@themsaid
Copy link
Member

themsaid commented Apr 8, 2019

You can do that in your own project if you want, this is a breaking change for no real gain.

@themsaid themsaid closed this Apr 8, 2019
@Max13
Copy link
Contributor

Max13 commented Apr 8, 2019

For what it worths, I personally see the current state (hard linking these 2 libs) as an inconsistency.

I would have preferred them to be handled by package.json, which would allow the few of us not being 100% connected all the time, to still be able to use the functionalities depending on these libs. While not bringing any major downside to others.

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.

3 participants