Skip to content

Source JavaScript modules from npm #6203

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

Closed
silverwind opened this issue Feb 27, 2019 · 8 comments
Closed

Source JavaScript modules from npm #6203

silverwind opened this issue Feb 27, 2019 · 8 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first. type/refactoring Existing code has been cleaned up. There should be no new functionality.

Comments

@silverwind
Copy link
Member

silverwind commented Feb 27, 2019

JavaScript modules are currently manually placed in public/vendor/plugins and updating them is not straightfoward as one needs to both manually copy new files and update the licences page. I'd suggest:

  • Populate public/vendor/plugins by adding all modules to package.json and copying the relevant files from node_modules in a make task.
  • Remove the JavaScript Licences page, I don't think it is actually useful and just cumbersome to update.
@techknowlogick
Copy link
Member

I'm in favour of moving to npm for JS modules (in fact I recently forked a project to do exactly this to it), however the JavaScript Licences page is a requirement that we provide a list of the licenses used (perhaps this could be autogenerated from package.json info of each project).

@techknowlogick techknowlogick added type/refactoring Existing code has been cleaned up. There should be no new functionality. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Feb 27, 2019
@silverwind
Copy link
Member Author

silverwind commented Feb 27, 2019

Yes, most npm packages have license info in their package.json. Would you think the license name and a link to the package homepage would suffice? I guess not all packages would feature a LICENSE file.

@silverwind
Copy link
Member Author

I'm thinking of going one step further and adding a minimal webpack build process. It could produce a single bundle.min.js that we commit to git.

@apricote
Copy link
Contributor

Committing the bundle.min.js will cause more conflicts in PRs, in the same way that it currently happens with the css bundle. Not a blocker, just something to think about.

@silverwind
Copy link
Member Author

Yeah, I don't like committing build files either. We could require Node.js and a Internet connection to compile both CSS and JS during the build.

@apricote
Copy link
Contributor

apricote commented Mar 1, 2019

The build server could generate the files and commit them for master branch builds.

Upsides:

  • No more conflicts in PRs that touch these files,
  • contributers that work on Go code won't need Node.js,
  • our security improves slightly, because people can not hide malicious code in the minified JS bundle.

Downsides:

  • Reviewers will have to build it locally

@CL-Jeremy
Copy link
Contributor

CL-Jeremy commented Mar 13, 2019

The build server could generate the files and commit them for master branch builds.

Does this also mean these files will be added to .gitignore, while CI builds will do git add --force? That'd liberate me from doing git checkout HEAD -- <files> whenever those files get generated.

@silverwind
Copy link
Member Author

This is in progress. Not all libs are sourceable from npm but some might be. We can handle them on a case-per-case basis, no need to keep this open.

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

No branches or pull requests

4 participants