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

fix: correct main field in package.json for CDN support #51

Merged
merged 3 commits into from
Dec 13, 2022
Merged

Conversation

manzt
Copy link
Collaborator

@manzt manzt commented Dec 12, 2022

Should fix #37

Google Colab and VSCode rely on loading the bundled extension (dist/index.js) from a CDN. I believe Colab uses unpkg, but others may use jsdelivr, etc.

CDNs generally defer to the main field of the package.json (since this is how Node's resolution algorithm works), but there are unofficial fields like unpkg, jsdelivr, module, browser, etc. It would be simple enough to make the main field dist/index.js to support most CDNs, but then that breaks jupyterlab extension build (which requires the main to be src/index.js). Fix one leak and another opens 🙃

This PR uses publishConfig – an formal field used by npm (and other package managers) to override properties in the package.json at publish-time (e.g., when running npm publish). This means main remains src/index.js during development and when running all build commands, and then is changed to dist/index.js only when publish to npm.

In my experience, this is the easiest fix - especially since the source code (js/src/index.js) isn't useable in any JS runtime nor would be used by others with a bundler since it's so specific for the Jupyter-like clients.

@manzt manzt requested a review from flekschas December 12, 2022 21:55
@flekschas
Copy link
Owner

I still can't get it to run in VSCode. Is there a magic trick to re-install the local version?

@flekschas
Copy link
Owner

E.g., same issue as before. But then again, I don't know if VSCode requires some explicit magic to happen.

Screen.Recording.2022-12-12.at.8.00.45.PM.mp4
@manzt
Copy link
Collaborator Author

manzt commented Dec 13, 2022

Did you release a new version to npm? I believe VSCode relies on loading from CDN, and the package.json must be updated there

@flekschas
Copy link
Owner

Okay. Let's hope things work :D

@flekschas flekschas merged commit 1deb24e into master Dec 13, 2022
@flekschas flekschas deleted the manzt/npm branch December 13, 2022 01:33
@manzt
Copy link
Collaborator Author

manzt commented Dec 13, 2022

hmmmm, it might actually be related to the how the amd module is defined? I see a 200 and request for https://cdn.jsdelivr.net/npm/jupyter-scatter@0.8.0/dist/index.js in the network tab in VSCode

@manzt
Copy link
Collaborator Author

manzt commented Dec 13, 2022

for reference:

  • jupyter-scatter dist/index.js
define("jscatter",["module","@jupyter-widgets/base"], ...)
  • anywidget dist/index.js
define(["@jupyter-widgets/base"], ...);
@flekschas
Copy link
Owner

Oh boy haha

@manzt
Copy link
Collaborator Author

manzt commented Dec 13, 2022

looking at thewebpack.config.js, the embed export creates the dist/index.js:

jupyter-scatter includes an additional library field which is missing from the cookiecutter, making it a named amd module. Perhaps we can try removing this so that the module isn't named and perhaps more portable:

https://requirejs.org/docs/whyamd.html#namedmodules

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