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

Review dependency management #3404

Closed
asmecher opened this issue Feb 19, 2018 · 13 comments
Closed

Review dependency management #3404

asmecher opened this issue Feb 19, 2018 · 13 comments
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.

Comments

@asmecher
Copy link
Member

Currently we have a few kinds of dependencies:

  • Composer (lib/pkp/composer.json)
  • node.js dependencies (package.json)
  • Git submodules

We need to make sure we're covering a few requirements...

  • Notification if a specified version is dangerous
  • A plan for staying modern

For example, #3392 illustrates the dangers of fully-specified version numbers (we should be using ^x.y.z instead).

Review dependencies to ensure they're not overspecifying version numbers, and figure out a strategy to make sure we're decently future-proofed.

@asmecher asmecher added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Feb 19, 2018
@NateWr
Copy link
Contributor

NateWr commented Mar 6, 2018

GitHub should notify us of any dangers in a package.json or composer.json file in a project root, presuming the dependency is popular enough to be observed by GitHub.

If the git submodules are third-party, we'll need something different. But I think most of our submodules are ours, right?

@asmecher
Copy link
Member Author

asmecher commented Mar 6, 2018

We don't use a lot of third-party submodules, but we do have in lib/pkp...

  • js/lib/jquery/plugins/spectrum
  • js/lib/pnotify
  • lib/swordappv2

I had thought that github would warn us about lib/pkp/composer.json, but I didn't get any notification for #3392. I wonder whether it's up to the package maintainer to flag older releases as dangerous?

@NateWr
Copy link
Contributor

NateWr commented Mar 7, 2018

It looks like GitHub only tracks Ruby and NPM dependencies: https://help.github.com/articles/about-security-alerts-for-vulnerable-dependencies/#githubs-security-alerts-for-vulnerable-dependencies

It looks like Spectrum is available as a npm module.

pnotify is also available as a npm module, but we'll have to check if the version is compatible with what we're using. I'd like to swap that out for something integrated well with our Vue.js system soon.

asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 22, 2018
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 22, 2018
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 22, 2018
asmecher added a commit to asmecher/ojs that referenced this issue Oct 22, 2018
@asmecher
Copy link
Member Author

Update JQuery/JQueryUI to the latest:

This requires a change to the default theme plugin as some of the Composer dependencies moved around. (Note @NateWr and @Vitaliy-1 -- this will probably need porting to other themes when it's ready for merging.)

It looks like github is having server trouble, so I'll watch for the web hooks to kick off the tests later this afternoon...

asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 22, 2018
asmecher added a commit to asmecher/ojs that referenced this issue Oct 22, 2018
asmecher added a commit that referenced this issue Oct 22, 2018
asmecher added a commit to pkp/ojs that referenced this issue Oct 22, 2018
asmecher added a commit to asmecher/omp that referenced this issue Oct 22, 2018
asmecher added a commit to pkp/omp that referenced this issue Oct 22, 2018
asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 22, 2018
@NateWr
Copy link
Contributor

NateWr commented Oct 23, 2018

The jQuery bump to 3.x may cause some knock-on effects. Looking over the upgrade guide, the following are probably worth checking:

  • $.parseJSON() is deprecated and we might want to change it to JSON.parse().
  • The tag-it library, the selectBox library, default theme, and the how-to-cite script use a data attribute with a dash, and the behaviour has changed.

@Vitaliy-1
Copy link
Collaborator

And tag-it library has another issue with jquery 3.0+ : aehlke/tag-it#370

@asmecher
Copy link
Member Author

@Vitaliy-1, I've manually patched tag-it to cover that issue. It's not an ideal solution, but will hold us until we replace tag-it with something else (which is already filed elsewhere).

asmecher added a commit to asmecher/pkp-lib that referenced this issue Oct 24, 2018
asmecher added a commit to asmecher/ojs that referenced this issue Oct 24, 2018
asmecher added a commit to asmecher/ojs that referenced this issue Oct 24, 2018
@asmecher
Copy link
Member Author

asmecher commented Nov 2, 2018

The tag-it-related issues should be dealt with here: #4208

@NateWr
Copy link
Contributor

NateWr commented Nov 8, 2018

I found one more issue that I think is related to the jQuery bump to v3.0. PR: pkp/ojs#2157

NateWr added a commit to NateWr/ojs that referenced this issue Nov 9, 2018
NateWr added a commit to NateWr/ui-library that referenced this issue Nov 13, 2018
@NateWr
Copy link
Contributor

NateWr commented Nov 13, 2018

@asmecher Here's an update for the jQuery dependency used in ui-library.

PR:
pkp/ui-library#26

NateWr added a commit to NateWr/ui-library that referenced this issue Nov 13, 2018
NateWr added a commit to NateWr/pkp-lib that referenced this issue Nov 13, 2018
@NateWr
Copy link
Contributor

NateWr commented Nov 13, 2018

And another PR that updates the dependencies when enable_cdn is on. I think this explains why I was running into issues with $.when().

PR:
#4233

@asmecher
Copy link
Member Author

Please go ahead! I think this probably isn't the last JQuery/JQueryUI version shoehorned in that we'll have to find and remove...

@asmecher
Copy link
Member Author

I think the specifics referred to here have been resolved over the last months, or filed elsewhere. Closing.

henriqueramos pushed a commit to henriqueramos/ui-library that referenced this issue Jan 20, 2022
henriqueramos pushed a commit to henriqueramos/ui-library that referenced this issue Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

3 participants