-
Notifications
You must be signed in to change notification settings - Fork 25
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
Upgrade to JupyterLab 4 #402
Conversation
5b3bb60
to
7a45f31
Compare
Good work! The change to the iterator value (as seen in the file browser method you modified) was the only required change I saw locally, as well. I couldn't get both JL3 and JL4 to work in the same code without breaking all sorts of TypeScript type protections. I checked out and built your code locally and found that it worked as expected with Lab 4.0.3. |
b2daf2e
to
53f1957
Compare
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.
Good job! Please find some comments below.
While best practices generally encourage making PRs as focused as possible and not mixing different types of changes, I don't see it as a big problem here, let's just be mindful of it and update the PR name to something like "Upgrade to JupyterLab 4, rework SchedulerHelper test class" for the sake of project history clarity.
Resolved per internal discussion yesterday morning
* remove unused dependency * upgrade dependencies to JL4 * fix type errors * add dev-install script * update yarn.lock * bump JL version in build and workflows * pre-commit * fix lint * fix yarn.lock checksums * bump ts-jest to v29 * upgrade to jest 29, typescript 4.3.0 * pull in required JupyterLab 4 test configuration * upgrade to React 18 to be compatible w/ JL4 * fix Build/test-isolated job * bump @jupyterlab/galata to latest, regenerate yarn.lock * try not deleting node * hardcode labextension name to fix Build/test_isolated job * call playwright directly via node to fix Build/integration-tests * use new JL4 galata config * fix and simplify E2E tests * remove outdated snapshots * add test:update script * add workflow_dispatch trigger to choose workflow branch * use jlpm when updating E2E snapshots * Update Playwright Snapshots --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
This will have to be in a new major release (v2), since it is a breaking change for existing JupyterLab 3 users. However, we still intend to backport pertinent features and bug fixes back to v1.x at our discretion.
Surprisingly, the only necessary changes to get our code to build with JL4 were the following:
skipLibCheck: true
intsconfig.json
src/index.tsx
These are implemented in 51c551a.
This begs the question of whether we should instead write the code in a manner that builds with both JupyterLab 3 & 4, and release two distributables:
jupyter_scheduler@2.0.0
(JL4) andjupyter_scheduler_jl3@2.0.0
(JL3). Our team is currently discussing this at the moment.