-
Notifications
You must be signed in to change notification settings - Fork 950
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
Binder and yarn tweaks #3795
Binder and yarn tweaks #3795
Conversation
Well, that did nothing. 3.6.4 is already preinstalled, so it's kept as-is. |
ipywidgets works with JupyterLab 3 and 4 |
Ah! Well, the explicitly not-matching versions of both jupyterlab and yarn seem bad, here...
|
Oh yes. It is indeed more subtle and binder may need JupyterLab 4 if it uses a dev version of ipywidgets. ipywidgets now requires to be built against JupyterLab 4. Once built and published on PyPi/conda-forge, it will work with both JupyterLab 3 and JupyterLab 4. Before #3752, ipywidgets required to be built against JupyterLab 3, but that does not mean the built and published package did not work with JupyterLab 4 (apart from a couple small issues it is working). So you're right, we should update the binder setup to use JupyterLab 4 if binder builds its own version from dev. |
I would be in favor of not using a dev version of ipywidgets on Binder though, and instead use the latest published version on either PyPi or conda-forge. Because it may be surprising to users to not get the same features in their local setup as in the binder. |
Binder from pull requests should probably build ipywidgets from the PR's source branch, though, no? Otherwise, that automatic comment with a link to a Binder notebook... running ipywidgets without any of the code from the PR... seems not entirely helpful/useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking of yarn invocations, apparently with Yarn 2+ it's... well, debatably a good idea, but at the very least "not wrong" (according to them, anyway) to check into the repo either:
- The Yarn version in use, so that everyone is running the same tools, or
- The entire dependency cache, iff it's in the form of compressed package archives rather than a giant, exploded
node_modules
hierarchy. That requires switching to using theirpnp
linker, instead of thenode-modules
one that's the default for projects coming from yarn 1.
Not necessities by any stretch, but apparently options possibly worth considering.
Yarn will reformat the file as it sees fit, so having prettier edit it as well creates a constant back-and-forth of style changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Sorry for the late review
(Separate) commits in this PR:
Upgrade jupyterlab in Binder from 3.x to 4.0.2
Run 'yarn set version stable' in Binder, to use yarn 3
Remove
yarn-deduplicate
, which is only compatible with yarn 1. (It's unnecessary with yarn 2+, anyway.)Adds
packageExtensions:
configs to the.yarnrc.yml
, to silence warnings about undeclared dependencies in imported packages. (Except for the fundamental incompatibility ofistanbul-instrumenter-loader
with webpack 5.) See Setting up repo with yarn 3 fails #3797 for more info.(Note: There's also a remaining undeclared
react
dependency injupyterlab/settingregistry
, but adding that to thepackageExtensions
makesreact
a dependency of the project as well, which was a road I decided not to go down.)Create a.prettierignore
file in the repo root, and add.yarnrc.yml
to it.Add a comment,
# prettier-ignore
to the section of
.yarnrc.yml
that Yarn will edit on its own. (Using a.prettierignore
file didn't work, because the scripting is already passing--ignore-file .gitignore
on the command line, and Prettier doesn't support multiple ignore files.)Yarn edits the
.yarnrc.yml
file automatically, and its own style rules conflict with Prettier's. The two are in constant tension if Prettier is also making changes. It's best to consider the file's contents as managed by Yarn.Additional
# prettier-ignore
comments may be needed as the file grows. The comment only disables Prettier for the single block of code immediately following it.