Misc changes

Don Jayamanne (DonJayamanne) · Apr 10, 2020 · Apr 2, 2020 · Apr 2, 2020 · Apr 2, 2020 · Apr 2, 2020
commit 2342dfcf16d769ebab0967cd040a4474b0f15ab0
diff --git a/src/datascience-ui/ipywidgets/container.tsx b/src/datascience-ui/ipywidgets/container.tsx
@@ -37,6 +37,8 @@ type NonPartial<T> = {
 export class WidgetManagerComponent extends React.Component<Props> {
     private readonly widgetManager: WidgetManager;
     private readonly widgetSourceRequests = new Map<string, Deferred<void>>();
+    private readonly widgetSourcesAlreadyRegistered = new Map<string, string | undefined>();
+    private timedoutWaitingForWidgetsToGetLoaded?: boolean;
     private readonly loaderSettings = {
         // Whether to allow loading widgets from 3rd party (cdn).
         loadWidgetScriptsFromThirdPartySource: false,
@@ -119,16 +121,39 @@ export class WidgetManagerComponent extends React.Component<Props> {
         const config: { paths: Record<string, string> } = {
             paths: {}
         };
+
+        const promisesToResolve: Deferred<void>[] = [];
         sources
+            .filter((source) => {
+                // Ignore scripts that have already been registered once before.
+                if (
+                    this.widgetSourcesAlreadyRegistered.has(source.moduleName) &&
+                    this.widgetSourcesAlreadyRegistered.get(source.moduleName) === source.scriptUri
+                ) {
+                    return true;
+                }
+                this.widgetSourcesAlreadyRegistered.set(source.moduleName, source.scriptUri);
+                return false;
+            })
             .map((source) => {
+                if (source.scriptUri) {
+                    // tslint:disable-next-line: no-console
+                    console.log(
+                        `Source for IPyWidget ${source.moduleName} found in ${source.source} @ ${source.scriptUri}.`
+                    );
+                } else {
+                    // tslint:disable-next-line: no-console
+                    console.error(`Source for IPyWidget ${source.moduleName} not found.`);
+                }
+
                 // We have fetched the script sources for all of these modules.
                 // In some cases we might not have the source, meaning we don't have it or couldn't find it.
                 let deferred = this.widgetSourceRequests.get(source.moduleName);
                 if (!deferred) {
                     deferred = createDeferred();
                     this.widgetSourceRequests.set(source.moduleName, deferred);
                 }
-                deferred.resolve();
+                promisesToResolve.push(deferred);
                 return source;
             })
             .filter((source) => source.scriptUri)
@@ -140,13 +165,13 @@ export class WidgetManagerComponent extends React.Component<Props> {
             });
 
         requirejs.config(config);
+        // Now resolve promises (anything that was waiting for modules to get registered can carry on).
+        promisesToResolve.forEach((item) => item.resolve());
     }
     private registerScriptSourceInRequirejs(source?: WidgetScriptSource) {
         if (!source) {
             return;
         }
-        // tslint:disable-next-line: no-console
-        console.log(`Source for IPyWidget ${source.moduleName} ${source.scriptUri ? 'Not found' : source.scriptUri}`);
         this.registerScriptSourcesInRequirejs([source]);
     }
     private createLoadErrorAction(
@@ -198,9 +223,24 @@ export class WidgetManagerComponent extends React.Component<Props> {
         if (!deferred) {
             deferred = createDeferred<void>();
             this.widgetSourceRequests.set(moduleName, deferred);
+
             // If we timeout, then resolve this promise.
             // We don't want the calling code to unnecessary wait for too long.
-            setTimeout(() => deferred?.resolve(), this.loaderSettings.timeoutWaitingForScriptToLoad);
+            // Else UI will not get rendered due to blocking ipywidets (at the end of the day ipywidgets gets loaded via kernel)
+            // And kernel blocks the UI from getting processed.
+            // Also, if we timeout once, then for subsequent attempts, wait for just 1 second.
+            // Possible user has ignored some UI prompt and things are now in a state of limbo.
+            // This way thigns will fall over sooner due to missing widget sources.
+            const timeoutTime = this.timedoutWaitingForWidgetsToGetLoaded
+                ? 1_000
+                : this.loaderSettings.timeoutWaitingForScriptToLoad;
+
+            setTimeout(() => {
+                // tslint:disable-next-line: no-console
+                console.error(`Timeout waiting to get widget source for ${moduleName}, ${moduleVersion}`);
+                deferred?.resolve();
+                this.timedoutWaitingForWidgetsToGetLoaded = true;
+            }, timeoutTime);
         }
         // Whether we have the scripts or not, send message to extension.
         // Useful telemetry and also we know it was explicity requestd by ipywidgest.
@@ -209,10 +249,9 @@ export class WidgetManagerComponent extends React.Component<Props> {
             { moduleName, moduleVersion }
         );
 
-        return (
-            deferred.promise
-                // tslint:disable-next-line: no-console
-                .catch((ex) => console.log('Failed to load Widget Script from Extension', ex))
+        return deferred.promise.catch((ex) =>
+            // tslint:disable-next-line: no-console
+            console.error(`Failed to load Widget Script from Extension for for ${moduleName}, ${moduleVersion}`, ex)
         );
     }
 }
diff --git a/src/ipywidgets/src/manager.ts b/src/ipywidgets/src/manager.ts
@@ -27,7 +27,6 @@ export class WidgetManager extends jupyterlab.WidgetManager {
         kernel: Kernel.IKernelConnection,
         el: HTMLElement,
         private readonly scriptLoader: {
-            readonly loadWidgetScriptsFromThirdPartySource: boolean;
             readonly widgetsRegisteredInRequireJs: Readonly<Set<string>>;
             errorHandler(className: string, moduleName: string, moduleVersion: string, error: any): void;
             loadWidgetScript(moduleName: string, moduleVersion: string): Promise<void>;
@@ -104,35 +103,22 @@ export class WidgetManager extends jupyterlab.WidgetManager {
                     widgetsRegisteredInRequireJs.includes(moduleName) ||
                     this.scriptLoader.widgetsRegisteredInRequireJs.has(moduleName);
 
-                if (!this.scriptLoader.loadWidgetScriptsFromThirdPartySource && !loadModuleFromRequirejs) {
-                    throw new Error('Loading from 3rd party source is disabled');
-                }
-
                 if (!loadModuleFromRequirejs) {
                     // If not loading from requirejs, then check if we can.
+                    // Notify the script loader that we need to load the widget module.
+                    // If possible the loader will locate and register that in requirejs for things to start working.
                     await this.scriptLoader.loadWidgetScript(moduleName, moduleVersion);
                 }
 
                 // If loading module from requirejs (e.g. already bundled), then do not use the cdn.
-                const useCdn = !loadModuleFromRequirejs && this.scriptLoader.loadWidgetScriptsFromThirdPartySource;
-                const m = await requireLoader(moduleName, moduleVersion, useCdn);
+                const m = await requireLoader(moduleName);
                 if (m && m[className]) {
                     return m[className];
                 }
                 throw originalException;
             } catch (ex) {
                 this.scriptLoader.errorHandler(className, moduleName, moduleVersion, originalException);
-                if (this.scriptLoader.loadWidgetScriptsFromThirdPartySource) {
-                    throw originalException;
-                } else {
-                    // Don't throw exceptions if disabled, else everything stops working.
-                    window.console.error(ex);
-                    // Returning an unresolved promise will prevent Jupyter ipywidgets from doing anything.
-                    // tslint:disable-next-line: promise-must-complete
-                    return new Promise(() => {
-                        // Noop.
-                    });
-                }
+                throw originalException;
             }
         });
 

diff --git a/src/ipywidgets/src/widgetLoader.ts b/src/ipywidgets/src/widgetLoader.ts
@@ -3,33 +3,6 @@
 
 'use strict';
 
-// This is the magic that allows us to load 3rd party widgets.
-// If a widget isn't found locally, lets try to get the required files from `unpkg.com`.
-// For some reason this isn't the default behavior of the html manager.
-
-// Source borrowed from https://github.com/jupyter-widgets/ipywidgets/blob/master/examples/web3/src/manager.ts
-
-const cdn = 'https://unpkg.com/';
-
-function moduleNameToCDNUrl(moduleName: string, moduleVersion: string) {
-    let packageName = moduleName;
-    let fileName = 'index'; // default filename
-    // if a '/' is present, like 'foo/bar', packageName is changed to 'foo', and path to 'bar'
-    // We first find the first '/'
-    let index = moduleName.indexOf('/');
-    if (index !== -1 && moduleName[0] === '@') {
-        // if we have a namespace, it's a different story
-        // @foo/bar/baz should translate to @foo/bar and baz
-        // so we find the 2nd '/'
-        index = moduleName.indexOf('/', index + 1);
-    }
-    if (index !== -1) {
-        fileName = moduleName.substr(index + 1);
-        packageName = moduleName.substr(0, index);
-    }
-    return `${cdn}${packageName}@${moduleVersion}/dist/${fileName}`;
-}
-
 // tslint:disable-next-line: no-any
 async function requirePromise(pkg: string | string[]): Promise<any> {
     return new Promise((resolve, reject) => {
@@ -42,21 +15,6 @@ async function requirePromise(pkg: string | string[]): Promise<any> {
         }
     });
 }
-export function requireLoader(moduleName: string, moduleVersion: string, useCdn: boolean) {
-    if (useCdn) {
-        // tslint:disable-next-line: no-any
-        const requirejs = (window as any).requirejs;
-        if (requirejs === undefined) {
-            throw new Error('Requirejs is needed, please ensure it is loaded on the page.');
-        }
-        const conf: { paths: { [key: string]: string } } = { paths: {} };
-        conf.paths[moduleName] = moduleNameToCDNUrl(moduleName, moduleVersion);
-        requirejs.config(conf);
-        // tslint:disable-next-line: no-console
-        console.log(`Using CDN to load Widget Module ${moduleName}, version ${moduleVersion}`);
-    } else {
-        // tslint:disable-next-line: no-console
-        console.log(`Not using CDN to load Widget Module ${moduleName}`);
-    }
+export function requireLoader(moduleName: string) {
     return requirePromise([`${moduleName}`]);
 }