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

if moduleName contains a slash, use a different module/path #1995

Merged

Conversation

@maartenbreddels
Copy link
Member

@maartenbreddels maartenbreddels commented Mar 13, 2018

This is useful if a package contains multiple widget extensions (I use this for ipysheet).
For instance, use:
_model_module = Unicode('ipysheet/renderer').tag(sync=True)
The embed manager will try got get ipysheet/renderer.js from the server its running on (which is a valid request, and useful for local testing), if that fails it will fetch:
https://unpkg.com/ipysheet@semver/dist/renderer.js

@vidartf
Copy link
Member

@vidartf vidartf commented Mar 13, 2018

Does this need to special case @my-namespace/mymodule ?

@maartenbreddels
Copy link
Member Author

@maartenbreddels maartenbreddels commented Mar 13, 2018

Indeed, good call. Do you see any other issues with this?

@jasongrout
Copy link
Member

@jasongrout jasongrout commented Mar 16, 2018

The only other issue I see with this is that it still inserts the dist/ - is there a way we don't have to hardcode that portion either? I don't see a way to have it nicely support both the other widget managers, but perhaps you have an idea?

@maartenbreddels
Copy link
Member Author

@maartenbreddels maartenbreddels commented Mar 16, 2018

In the notebook this works without issues, requirejs handles it without issues, Jupyter lab also doesn't mind the extra slash in the registry. I think the 'dist/' part is ok, I see it as a manager specific implementation.

@jasongrout
Copy link
Member

@jasongrout jasongrout commented Mar 16, 2018

I think the 'dist/' part is ok, I see it as a manager specific implementation.

Oh right - it is very specific to how the html manager is fetching things.

@jasongrout
Copy link
Member

@jasongrout jasongrout commented Mar 16, 2018

Looks good to me, then. Thanks!

@jasongrout jasongrout merged commit 49edada into jupyter-widgets:master Mar 16, 2018
1 check passed
@jasongrout jasongrout added this to the Minor release milestone Mar 16, 2018
@jasongrout jasongrout removed this from the Minor release milestone Mar 19, 2018
@jasongrout jasongrout added this to the 7.2 milestone Mar 19, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants