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

resolve yarn.lock, bump builder version, some packaging metadata #307

Merged
merged 6 commits into from
Nov 29, 2021

Conversation

bollwyvl
Copy link
Collaborator

Not knowing/using jupyter-packaging much, I did the following:

cd jupyterlab-server-proxy
rm yarn.lock
jlpm
jlpm build:prod

And seemed to have a working extension. I also bumped the version of @jupyterlab/builder to be of the line that generates third-party-licenses.json: as it turns out, it only ships the shims for css-loader and style-loader. As the style/index.css is actually empty, we could ship with no dependencies whatsoever without losing any functionality, which is actually quite attractive.

Apropos #298, I restored one old-school compatibility name to make jupyter serverextension enable work.

I had some pip issues (due to my environment no doubt), but rolled back any changes there except adding the relatively-new Lab trove classifiers. I've added, but commented out JupyterLab :: 2... if no longer tested, and nobody's crying out for it, I don't recommend we re-add it to the test matrix.

Here's hoping CI is kind!

@todo
Copy link

todo bot commented Nov 29, 2021

JupyterLab 2 is theoretically supported, but is not tested

# TODO: JupyterLab 2 is theoretically supported, but is not tested
# "Framework :: Jupyter :: JupyterLab :: 2",
"Framework :: Jupyter :: JupyterLab :: 3",
"Framework :: Jupyter :: JupyterLab :: Extensions",
"Framework :: Jupyter :: JupyterLab :: Extensions :: Prebuilt",
"Operating System :: Microsoft :: Windows",


This comment was generated by todo based on a TODO comment in 4f49a2f in #307. cc @bollwyvl.

@bollwyvl
Copy link
Collaborator Author

Happy to make any other changes, or add docs/explanations...

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wieee this LGTM! THANK YOU @bollwyvl !!!!!! ❤️ 🎉 🌻

@yuvipanda @manics @ryanlovett, I figured I'd squeeze this change into 3.2.0 to get rid of a security warning.

It would feel nice to have another approval of this, because I don't understand the JS world very well, and since I'd like to include it in the 3.2.0 release.

@ryanlovett
Copy link
Collaborator

@consideRatio Thanks for waiting for this to merge before release! I defer to the others as well since I feel the same about JS. If there’s no activity after a long period then I can research it, though I’m guessing someone else will chime in.

@consideRatio
Copy link
Member

Btw, I've looked into the security alerts and if they are solved by this yarn.lock update - sadly, they are not. It seems that the pinnings of @jupyterlab/application and its dependencies are too stringent and still referencing the packages with vulnerabilities.

image

Hmmm, but wait, it seems like @jupyterlab/application is at https://www.npmjs.com/package/@jupyterlab/application rather than 3.0.7 as we have it in this yarl.lock file.

@bollwyvl was the versions resolved to in yarl.lock in this PR recently updated? Do you understand what held back @jupyterlab/application from resolving to something more modern, like 3.2.4 than the 3.0.7 that we got? I note that there are even versions like 3.0.12 out, so it seems strange if we ended up with 3.0.7.

@bollwyvl
Copy link
Collaborator Author

I've re-pushed, replacing the ^ with ~ operator on the various ranges, and tweaked until I could get it to:

  • build the typescript
  • build the extension
  • not complain about audits with jlpm audit

That being said: it's no secret that I despise the crazy version operators. I am not the first person to misunderstand them, and guarantee I won't be the last, so these might be wrong, but a look at the delta to the yarn.lock seems appropriate.

@manics
Copy link
Member

manics commented Nov 29, 2021

https://stackoverflow.com/questions/22343224/whats-the-difference-between-tilde-and-caret-in-package-json
Shouldn't it be ^ instead of ~ to support JupyterLab 3.*, not just 3.0.*?

@bollwyvl
Copy link
Collaborator Author

Sure, let's try it with ^2.0 || ^3.0.

@bollwyvl
Copy link
Collaborator Author

I generally drop stuff into an online checker, e.g.

https://jubianchi.github.io/semver-check/#/constraint/^2.0%20||%20^3.0

@bollwyvl
Copy link
Collaborator Author

As, on closer inspection, we are running the tests, I've put the trove classifier back in.

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wieeeeeeeeeeeeeeeeeeeeeee!!! Thank you so much @bollwyvl for your thorough work with this!!!!! ❤️ 🎉

This LGTM - go for merge? If so, I'll go for a release of 3.2.0 including this finally - now with the security warnings resolved I think.

@maresb
Copy link
Contributor

maresb commented Nov 29, 2021

I'm really looking forward to this release!!! I wanted rewrite_response well over a year ago. And coincidentally just right now I have another important use case for it. So perfect timing! 😄

Copy link
Collaborator Author

@bollwyvl bollwyvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@consideRatio
Copy link
Member

Aaaaahhh I'll go for a merge and release since I'm at the computer ready to do it right now and I think there is quite clear consensus on this change and to cut the release 3.2.0 with this added to it.

@consideRatio consideRatio merged commit 3862b99 into jupyterhub:master Nov 29, 2021
@ndgayan
Copy link

ndgayan commented Nov 29, 2021

I'm so excited about the new release. Could you also publish the updated version to pypi.org ASAP?

@bollwyvl bollwyvl deleted the gh-306-update-yarn-lock branch November 29, 2021 23:30
@consideRatio
Copy link
Member

@ndgayan sure can, it is available on PyPI now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating the yarn.lock file: to not include security warnings and be generally up to date
6 participants