Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Use webpacker for JS dependencies #2126

Closed
grzuy opened this issue Sep 6, 2019 · 5 comments
Closed

Use webpacker for JS dependencies #2126

grzuy opened this issue Sep 6, 2019 · 5 comments

Comments

@grzuy
Copy link
Contributor

grzuy commented Sep 6, 2019

I think it would be good to have webpacker/webpack for managing and updating JS depedencies more easily.

@grzuy grzuy changed the title Use webpack for JS dependencies Use webpacker for JS dependencies Sep 6, 2019
@grzuy
Copy link
Contributor Author

grzuy commented Sep 13, 2019

Do others agree this would be a good idea?

If so I could open PR with this.

@grzuy grzuy mentioned this issue Oct 7, 2019
18 tasks
@sonalkr132
Copy link
Member

managing and updating JS depedencies more easily.

we don't have any js dependency beside jquery (which can probably be removed as well). No plans or need for ES6 either.
is this still useful?

@sonalkr132
Copy link
Member

Hi, I am closing this. Feel free to re-open if you think this would be of significant help for us.

@grzuy
Copy link
Contributor Author

grzuy commented Apr 19, 2020

Hi @sonalkr132,

Thanks for the feedback.

managing and updating JS depedencies more easily.

we don't have any js dependency beside jquery (which can probably be removed as well).

As far as I can see there's also 2 extra JS dependencies in vendor/assets/javascripts:

No plans or need for ES6 either.
is this still useful?

Apart from the usefulness of keeping the existing dependencies up to date more easily, opening this issue was also motivated by the fact that I was having trouble introducing a new JS dependency in the WebAuthn pull request (#2108 (comment)). The complication was due to the fact that @github/webauthn-json, isn't even considering people copy-pasting their source code in their apps as an expected installation dev flow so they don't provide dist files. So one needs to build/generate it's own dist file, which is a bit suboptimal and in some way error prone, compared to npm install @github/webauthn-json or any other future JS depdenency.

Not a huge thing - but wanted to give more details behind my original motivation if helpful.

Thanks again.

@sonalkr132
Copy link
Member

keeping the existing dependencies up to date more easily

this is valid point. I think our intention with vendoring everything is to avoid depending on third party system for build/deployment.
We should have definitely kept clipboard up to date, and manual copy paste is good enough as long as we had only one dependency to update. At this point, github-buttons is "our" code given that upstream hasn't had udpates in 5 years and we have made quite a few changes to it.
I am not sure where we stand with adding one more JS dependency and nodejs/npm. Traditionally we have been very conservative about adding dependencies to our app.

I guess this should be open for discussion.

@sonalkr132 sonalkr132 reopened this Apr 19, 2020
@rubygems rubygems locked and limited conversation to collaborators Jun 14, 2022
@hsbt hsbt converted this issue into discussion #3098 Jun 14, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

2 participants