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

Service worker fixes, JupyterLab 3.5.1 #899

Merged
merged 45 commits into from
Dec 12, 2022

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Dec 8, 2022

References

Code changes

  • add exact-pinned pyodide to pyodide-kernel/package.json#/devDependencies
    • only ever import typed in .ts
    • is the source of truth for all other versions
  • bump to pyodide 0.22.0a3

  • add @types/emscripten (such as it is)
  • remove duplicated ERRNO_CODES
  • remove use of private pyodide._module
  • update key dev deps
    • lerna 6
    • typedoc 0.23
    • typescript 4.9
    • prettier 2.8
  • refactor BroadcastChannel out of server, put in contents
  • make pyolite more resilient to missing ServiceWorker and BroadcastChannel
  • moves services.js into server/sw.ts
    • add a new webpack plugin to hoist it up to the root of the app (or the scope is wrong) and add the content hash to the file name
  • add SKIP_ESLINT env var
    • the pre-commit hooks still fail sometimes, so git commit -n it shall be!

  • remove plotly until compatible with ipywidgets 8

User-facing changes

  • site deployers
    • can disable the ServiceWorker
  • end users
    • should get fewer stale serves of services.js with cache-busting

Backwards-incompatible changes

  • some Emscripten-related types and classes have moved
  • the name of services.js is no longer predictable (hash-based)

@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2022

lite-badge 👈 Try it on ReadTheDocs

@martinRenou
Copy link
Member

Headsup that this will go in conflict with #898

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Dec 8, 2022 via email

@bollwyvl bollwyvl added enhancement New feature or request maintenance kernel:serviceworker ServiceWorker-related challenges labels Dec 8, 2022
@stevejpurves
Copy link
Member

stevejpurves commented Dec 8, 2022

couple of questions:

How can a site deployer disable the service worker? should we expect to be able to set a new option in the jupyter-config-data?

... and add the content hash to the file name

So this is the content hash of the sw.ts file and will remain stable across releases/builds otherwise?

@bollwyvl
Copy link
Collaborator Author

bollwyvl commented Dec 8, 2022

How can a site deployer disable the service worker?

added some docs here: ada7479

the content hash of the sw.ts file and will remain stable across releases/builds otherwise?

I believe it's the hash of the output, which isn't precisely deterministic (or rather, has many independent variables of which the source of sw.ts is only one). A new sw-{hash}.js will only (but very possibly) change with a new release of jupyterlite 0.1.0b{n+1}. Two successive runs of jupyter lite build without pip installing something will always have the same sw-{hash}.js.

@@ -0,0 +1,154 @@
// eslint-disable-next-line @typescript-eslint/triple-slash-reference
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could rename this file from sw.ts to serviceworker.ts so it's more explicit?

And the existing serviceworker.ts to serviceworkerwrapper.ts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

perhaps serviceworker.ts and servicemanager.ts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

or service-registration.ts

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

3a39e7f went with ServiceManager, etc

Copy link
Member

Choose a reason for hiding this comment

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

Thanks. Although ServiceManager can probably be a bit confusing since there is already a ServiceManager in JupyterLab?

Copy link
Member

Choose a reason for hiding this comment

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

ServiceWorkerManager would be fine I guess?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Excellent point, updated in 0bf86c2.

@jtpio jtpio added this to the 0.1.0 milestone Dec 9, 2022
Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@qqdaiyu55
Copy link
Contributor

@jtpio With this upgrade, we should include your fix: jupyterlab/jupyterlab#12849.
However I checked the bundle file package/build/7941.dc36874.js of jupyterlite v0.1.0-b16, it does not include your change. Did I miss anything here?
image

@jtpio
Copy link
Member

jtpio commented Jan 3, 2023

@qqdaiyu55 I think jupyterlab/jupyterlab#12849 was not backported to 3.5 so it's not available in JupyterLite.

We should however be able to pick it up when updating to the JupyterLab 4 packages in #826.

@qqdaiyu55
Copy link
Contributor

qqdaiyu55 commented Jan 3, 2023

@jtpio Thanks for reply. I'm implementing a custom drive and try to get it work for jupyterlite, and see the same issue reported here: jupyterlab/jupyterlab#12755.
Uncaught (in promise) Error: ContentsManager: renaming files must occur within a Drive

From my understanding, for any custom drive extension (e.g. jupyterlab-github), I should see the same renaming error when I opened a new document in root directory (e.g. CustomDrive:test.txt), if we didn't pick the fix into jupyterlite?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kernel:serviceworker ServiceWorker-related challenges maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants