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

Bump yarn to 3.6.4 #15295

Merged
merged 3 commits into from
Nov 14, 2023
Merged

Conversation

fcollonval
Copy link
Member

@fcollonval fcollonval commented Oct 24, 2023

References

Maintenance to bump to the latest Yarn v3+

I stayed away from the new v4. We may try it after JupyterLab 4.1

Code changes

Bump vendored yarn to 3.6.4

User-facing changes

None

Backwards-incompatible changes

None

@fcollonval fcollonval added this to the 4.1.0 milestone Oct 24, 2023
@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@bollwyvl
Copy link
Contributor

I stayed away from the new v4. We may try it after JupyterLab 4.1

While the performance benefits of 4 are tempting, we'd need some canary testing of a number of complex downstreams to assess whether this buys anything valuable... the transition from 1 to 3 still hasn't demonstrated a lot of value to me, as we upgraded in an entirely-breaking way (CLI, lockfile, etc), but didn't take many of its new opinions which might have contained those benefits (zero install).

More broadly: still in favor of removing the jlpm blob (and jupyter lab(extension) build) entirely from the user-facing package which provides increasingly less value to users as the maintainer community makes less use of jupyter lab build.

It would be worth it to keep it in a super-cut-down jupyterlab-builder PyPI package (not hatch-jupyterlab-builder), suitable for use in a build-system, which would allow for potentially supporting multiple target versions of jupyterlab, which would act as a more resilient bridge.

@krassowski
Copy link
Member

Something else to note here - with immutable builds the downstream extensions will need to refresh lock files after updating JupyterLab (even if they don't bump any npm dependencies). Not a huge pain but I saw reports of "this lockfile is different for me locally" because an earlier point release of yarn was shipped in JupyterLab RC phase.

@fcollonval
Copy link
Member Author

fcollonval commented Oct 27, 2023

Something else to note here - with immutable builds the downstream extensions will need to refresh lock files after updating JupyterLab (even if they don't bump any npm dependencies). Not a huge pain but I saw reports of "this lockfile is different for me locally" because an earlier point release of yarn was shipped in JupyterLab RC phase.

Yes the reason for it is the yarn typescript patch; I wish it was not needed or somehow they don't complain about the version in the lock file as it is builtin within yarn.

I'll add a note in the migration guide.

@krassowski
Copy link
Member

For anyone interested: yarnpkg/berry@@yarnpkg/cli/3.5.0...@yarnpkg/cli/3.6.4 there are a few performance improvements and a few bugfixes.

@fcollonval fcollonval merged commit 5a28ab4 into jupyterlab:main Nov 14, 2023
76 of 78 checks passed
@fcollonval fcollonval deleted the maintenance/bump-yarn branch November 14, 2023 12:26
@jtpio
Copy link
Member

jtpio commented Dec 1, 2023

More broadly: still in favor of removing the jlpm blob entirely from the user-facing package

Curious: removing the jlpm blob just from the user facing package, or entirely?

If it now becomes tedious for extension authors relying on jlpm to update their yarn.lock on minor versions, I'm wondering if we wouldn't be better off dropping jlpm entirely, and let extension authors choose whatever they want (npm, pnpm or something else).

Happy to open an issue to discuss that more broadly if there is interest.

@bollwyvl
Copy link
Contributor

bollwyvl commented Dec 1, 2023

update their yarn.lock on minor versions

They have to do this anyway if rebuilding against a new version?

At any rate, getting it (and all code related to nodejs) out of the jupyterlab 5.x distribution altogether, and into a dedicated [build-system]-friendly package, would be magnificent for 5.x, as 99% of users don't need it anymore.... i thought there was an issue, but can't find it.

@bollwyvl
Copy link
Contributor

bollwyvl commented Dec 1, 2023

Ah, #11336

@krassowski
Copy link
Member

They have to do this anyway if rebuilding against a new version?

The point is that this breaks CI even if not rebuilding nor updating versions (unless you pinned your jupyterlab to 4.0), see jupyter/notebook#7170 as an example.

@jtpio
Copy link
Member

jtpio commented Dec 1, 2023

The point is that this breaks CI even if not rebuilding nor updating versions (unless you pinned your jupyterlab to 4.0), see jupyter/notebook#7170 as an example.

Yes. In that case notebook defines jupyterlab >=4.1.0a3, but was then getting jupyterlab >=4.1.0a4 with the new yarn, which broke CI.

@jtpio
Copy link
Member

jtpio commented Dec 1, 2023

Ah, #11336

Yes agree this should be on the radar for 5.0.0.

But here the question was a bit more narrow and focused on dropping jlpm entirely.

@bollwyvl
Copy link
Contributor

bollwyvl commented Dec 1, 2023

So, in the near term, perhaps we roll back this change. I still haven't heard the compelling reason to use .4, and breaks notebook is a strong mark against.

dropping jlpm entirely

I would maintain that having a working, (apparently-not-very) tested system available is, and will continue to be, crucial.

I blinked, so may have missed the new js package manager that Fixes All The Problems. Provided that didn't happen, and the opinion of the week rules, then we would inherit supporting all of the problems with all the systems on all the operating systems for all of the users that happen to type jupyter labextension install.

Thus far, we have at least partially mitigated this by going through the python-declared entry_points route with a white-label installer that relies on:

  • the existence of nodejs on $PATH
  • a populated node_modules
  • something that respects package.json#/resolutions
  • a stable lockfile (but guess not, right?)

... none of which are guaranteed by any of the other systems.

A potential solution, even in 4.1, is to offer an expansion of the schema to eject from all the jlpm defaults, which might look something like, via dumb-old-env-vars:

{
  "jupyterlab": {
    "builder": {
      "scripts": {
        "install": ["${JLAB_NODEJS}", "${JLAB_YARN}"],
        "build": ["${JLAB_NODEJS}", "${JLAB_BUILDER_JS}"],
        "watch": ["${JLAB_NODEJS}", "${JLAB_YARN}", "watch"],
      },
      "needs": {
        "install": ["${JLAB_NODEJS}", "${JLAB_YARN}"],
        "build": ["${JLAB_NODEJS}", "${JLAB_YARN}", "${JLAB_BUILDER_JS}"],
        "watch": ["${JLAB_NODEJS}", "${JLAB_YARN}", "${JLAB_BUILDER_JS}"],
      }
    }
  }
}

If someone has a toolchain that accepts the same arguments as @jupyterlab/builder (--core-path, etc), and generates all the correct package.json metadata in the right places after the fact, without rebundling an incompatible version of @lumino/widgets, go to town!

@krassowski
Copy link
Member

So, in the near term, perhaps we roll back this change

I am not strongly opposing, but...

breaks notebook is a strong mark against.

is not strictly correct. It does not break notebook. It does not break anything in the user land. It will be an annoyance for the extension authors who use immutable installs when released because it will break the CI (until the lockfile is updated).

jtpio added a commit that referenced this pull request Dec 13, 2023
jtpio added a commit that referenced this pull request Dec 14, 2023
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.

4 participants