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

load plotly.js from unminified dist bundle in plotlywidget #1891

Closed
wants to merge 1 commit into from

Conversation

jonmmease
Copy link
Contributor

With this PR, both jupyterlab-plotly and plotlywidget now load plotly.js from the unminified dist bundle, so jupyterlab should be able to de-duplicate plotly.js when both extensions are installed.

Closes #1873

cc @vidartf

@jonmmease jonmmease added this to the v4.3.0 milestone Nov 11, 2019
@emmanuelle
Copy link
Contributor

So the index.js file is now larger (7 Mo instead of 4). Is this a problem (for other environments, eg Dash applications etc.). Probably a naive question :-)

@jonmmease
Copy link
Contributor Author

@nicolaskruchten any thoughts on this. We could use the min bundle in both extensions instead of the regular one. My thinking was that this should be getting compressed everywhere it matters, but I don't have a strong opinion here.

@jonmmease jonmmease modified the milestones: v4.3.0, v4.4.0 Nov 11, 2019
@nrbgt
Copy link

nrbgt commented Nov 19, 2019

Perhaps when making this change, please also consider registering the exports in the lab plugin with a lazy load: this will keep it from

  • hitting the browser until it's actually requested
  • getting compiled in with the already-titanic vendor~main

Here's how it interacts with the crucial loadClass: https://github.com/jupyter-widgets/ipywidgets/blob/master/packages/jupyterlab-manager/src/manager.ts#L383

Since it appears to be commonjs, you may have to use the vendor-specific webpack.ensure (or require.ensure?) rather than the standards-based await import. But doing it at the widget load level is far more convenient than trying to do it within the backbone-related code, which doesn't async much.

@nrbgt
Copy link

nrbgt commented Nov 19, 2019

Here's an example with jupyter-threejs:

jupyter-widgets/pythreejs#297

@vidartf
Copy link

vidartf commented Nov 20, 2019

@nrbgt While I agree with your point, I believe it should be made as a separate issue.

@nrbgt
Copy link

nrbgt commented Nov 20, 2019

@vidartf made issue #1913

@nicolaskruchten nicolaskruchten modified the milestones: v4.4.0, v4.3.1 Nov 22, 2019
@jonmmease jonmmease modified the milestones: v4.3.1, v4.5.0 Dec 10, 2019
@nicolaskruchten
Copy link
Contributor

OK so as I understand it right now, this will bloat the nbextension size because we're not minifying in webpack.config.js.

@nicolaskruchten nicolaskruchten deleted the extension_plotlyjs_version branch June 19, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Effect on JupyterLab bundle size
5 participants