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 package should exclude external, build & .github directories #2075

Closed
fknittel opened this issue May 5, 2022 · 6 comments
Closed

npm package should exclude external, build & .github directories #2075

fknittel opened this issue May 5, 2022 · 6 comments

Comments

@fknittel
Copy link

fknittel commented May 5, 2022

The npm package 'jquery-ui' includes the directory external/jquery/ (among others) in addition to the regular package dependency on jquery. Is there a reason for that?

Dropping the jquery copies would shrink the npm package by a few MBytes (~12 MiB, which is quite substantial, considering the whole package is around 15 MiB). Another benefit would be that security scanners no longer identify jquery-ui as containing vulnerable versions of jquery.

I would suggest to at least exclude the external/jquery*/ directories from NPM by listing it in .npmignore. Maybe even the complete external/ directory tree.

@mgol
Copy link
Member

mgol commented May 11, 2022

Thanks for the report.

I was worried the https://github.com/jquery/download.jqueryui.com code installs jquery-ui from npm, but no, it clones the repo. Considering that the tests & demos folders are already excluded, I don't see a reason to keep external. We could try removing it.

@fnagel do you know a reason for not excluding this folder? I also wonder if removing it should be treated as a breaking change and deferred until 1.14.

@fnagel
Copy link
Member

fnagel commented May 16, 2022

@mgol No, I have no knowledge why this folder is included. That was long before my time and I was never involved in the whole build and publishing thing.

I would consider removing this folder as breaking change. One could use the files in local building tools, so removing those would be breaking.

@mgol
Copy link
Member

mgol commented May 17, 2022

Thanks, Felix! I was also leaning towards treating this as breaking but I wanted a second opinion.

Setting the milestone appropriately.

@mgol mgol added this to the 1.14.0 milestone May 17, 2022
@devicenull
Copy link

Did anything ever come of this? Having all the old jquery versions in external/ is setting off our security scans.

@mgol
Copy link
Member

mgol commented Jun 8, 2023

We consider this a breaking change which is why it’s scheduled for 1.14.0. We don’t expect that version to arrive soon, though.

@mgol
Copy link
Member

mgol commented Jul 11, 2023

We should also remove the build & .github folders from the package, BTW.

@mgol mgol changed the title npm package should exclude external/jquery* Jul 11, 2023
@mgol mgol closed this as completed in f849c50 May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment