Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Added lazy loading to jupyterlab-plotly - #2260

Closed
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly
Closed

Added lazy loading to jupyterlab-plotly#2260
pragyagarg642 wants to merge 7 commits into
plotly:masterfrom
pragyagarg642:lazyload_plotly

Conversation

@pragyagarg642

Copy link
Copy Markdown

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.

Fixes#1913

With this change, plotly.js will be loaded on demand. This uses dynamic
imports to achieve this.
Fixesplotly#1913
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Yes, I will take a look. A couple of questions/concerns though: right now in JupyterLab we need to install both jupyterlab-plotly and plotlywidget and both need plotly.js loaded. Right now they both statically load the same bundle and webpack deduplicates it. Is there any way that your change could include plotlywidget as well? If not, I fear the bundle size will remain the same and we will lazy-load plotly.js.

It might be that we need to first merge the two extensions together and then make this change, but I'm not sure.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I'm working on the changes.

@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I've made changes according to your comment. Can you please review this?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

Thanks for these changes! We're quite busy preparing for the 4.6.0 release of Plotly.py but I will review this during the 4.7.x cycle.

@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , as the 4.6.0 release is complete, can you please review this?

@nicolaskruchtennicolaskruchten added this to the 4.7.0 milestone Apr 27, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

OK, I've had a chance to take a look at this PR, thanks again for submitting it! The TypeScript doesn't actually compile for me with this PR as-is, but with a few adjustments it does, and sure enough, the bundle is nicely split, loads when needed, deduplicates between the two extensions etc. I'll need to fork this PR to add my changes such that things compile, and then the rest of my team can do some QA on Windows and Linux etc, but just wanted to let you know that unless anything weird crops up, we'll be able to merge this for 4.7.

cc @emmanuelle@jonmmease

@nicolaskruchtennicolaskruchten modified the milestones: 4.7.0, 4.8.0May 1, 2020
@nicolaskruchten

Copy link
Copy Markdown
Contributor

Update: we need to release 4.7 on Monday to meet some internal deadlines, so I'm bumping this to 4.8, which is only about 10 days away!

This was referenced May 6, 2020
@pragyagarg642

pragyagarg642 commented May 13, 2020

Copy link
Copy Markdown
Author

Hi @nicolaskruchten , prefetch option can also be added to this change. With that option, module will be fetched just after the initial page load is completed and when required, module will be loaded from prefetch cache, making it faster. Without this option, it will take time to load. I can update the PR with this change as well.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

@pragyagarg642 sure, let's see what prefetching looks like! BTW I had to make these changes #2454 to get your branch to work, if you want to incorporate them... Otherwise TypeScript compilation failed.

I should add that in order to merge this, we'll have to make equivalent changes to https://github.com/plotly/jupyterlab-chart-editor as well, as people frequently install that one too, and we want to keep all the bundle-loading logic the same. I might move that code into this repo in the very near future to keep things in sync.

I'm sorry it's taking so long to get this merged in!

@nicolaskruchtennicolaskruchten modified the milestones: 4.8.0, 4.9.0May 14, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

@nicolaskruchten I updated the branch with required prefetch changes.

@nicolaskruchtennicolaskruchten modified the milestones: 4.9.0, 4.10Jul 9, 2020
@nicolaskruchtennicolaskruchten modified the milestones: 4.10, v4.xSep 10, 2020
@pragyagarg642

Copy link
Copy Markdown
Author

Hi @nicolaskruchten, is there any update on this or is there anything pending on my end?

@nicolaskruchten

Copy link
Copy Markdown
Contributor

See my comment in #2454 ... the bottleneck here is just my time in being able to QA this to the point where we're willing to roll this change out to all of our users. I'm sorry I haven't been able to find the time but this is potentially quite a risky change.

@nicolaskruchten

Copy link
Copy Markdown
Contributor

I do apologize about how this PR was left unattended for so long, but it was superseded by #3142 which was just merged, so the functionality will make it into the next version of the library.

Sign up for freeto 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.

Lazy load strategy for plotly bundle

2 participants

@pragyagarg642@nicolaskruchten