From 962140981acb91305d23f3a2ccdc0e3ddfd74fc4 Mon Sep 17 00:00:00 2001
From: Hood Chatham <hood@mit.edu>
Date: Sat, 22 May 2021 21:54:27 -0400
Subject: [PATCH] API Don't expose pyodide as a global variable in loadPyodide
 (#1597)

---
 Makefile                                |  4 +--
 conftest.py                             |  4 ++-
 docs/development/new-packages.md        |  2 +-
 docs/project/changelog.md               |  5 ++++
 docs/usage/loading-packages.md          |  8 +++---
 docs/usage/quickstart.md                | 16 ++++++------
 docs/usage/webworker.md                 |  2 +-
 pyodide-build/pyodide_build/buildpkg.py |  2 +-
 src/js/load-pyodide.js                  |  4 +--
 src/js/pyodide.js                       | 33 ++++++++++++-------------
 src/templates/console.html              |  2 +-
 src/webworker.js                        |  2 +-
 12 files changed, 46 insertions(+), 38 deletions(-)

diff --git a/Makefile b/Makefile
index 1c35e2d9c..0d5d45fa9 100644
--- a/Makefile
+++ b/Makefile
@@ -175,7 +175,7 @@ build/test.data: $(CPYTHONLIB)
 	)
 	( \
 		cd build; \
-		python $(FILEPACKAGER) test.data --lz4 --preload ../$(CPYTHONLIB)/test@/lib/python$(PYMINOR)/test --js-output=test.js --export-name=globalThis.pyodide._module --exclude __pycache__ \
+		python $(FILEPACKAGER) test.data --lz4 --preload ../$(CPYTHONLIB)/test@/lib/python$(PYMINOR)/test --js-output=test.js --export-name=globalThis.__pyodide_module --exclude __pycache__ \
 	)
 	$(UGLIFYJS) build/test.js -o build/test.js
 
@@ -188,7 +188,7 @@ build/distutils.data: $(CPYTHONLIB)
 	)
 	( \
 		cd build; \
-		python $(FILEPACKAGER) distutils.data --lz4 --preload ../$(CPYTHONLIB)/distutils@/lib/python$(PYMINOR)/distutils --js-output=distutils.js --export-name=pyodide._module --exclude __pycache__ --exclude tests \
+		python $(FILEPACKAGER) distutils.data --lz4 --preload ../$(CPYTHONLIB)/distutils@/lib/python$(PYMINOR)/distutils --js-output=distutils.js --export-name=globalThis.__pyodide_module --exclude __pycache__ --exclude tests \
 	)
 	$(UGLIFYJS) build/distutils.js -o build/distutils.js
 
diff --git a/conftest.py b/conftest.py
index 49566ad8e..39acbe7ad 100644
--- a/conftest.py
+++ b/conftest.py
@@ -98,7 +98,9 @@ class SeleniumWrapper:
         self.driver.get(f"http://{server_hostname}:{server_port}/test.html")
         self.javascript_setup()
         if load_pyodide:
-            self.run_js("await loadPyodide({ indexURL : './', fullStdLib: false });")
+            self.run_js(
+                "window.pyodide = await loadPyodide({ indexURL : './', fullStdLib: false });"
+            )
             self.save_state()
         self.script_timeout = script_timeout
         self.driver.set_script_timeout(script_timeout)
diff --git a/docs/development/new-packages.md b/docs/development/new-packages.md
index 820a3b846..b668773ea 100644
--- a/docs/development/new-packages.md
+++ b/docs/development/new-packages.md
@@ -245,7 +245,7 @@ We invoke it as follows:
 ```sh
 $ ./file_packager.py PACKAGE_NAME.data \
      --js-output=PACKAGE_NAME.js \
-     --export-name=pyodide._module \
+     --export-name=globalThis.__pyodide_module \
      --use-preload-plugins \
      --preload /PATH/TO/LIB/@/lib/python3.8/site-packages/PACKAGE_NAME/ \
      --exclude "*__pycache__*" \
diff --git a/docs/project/changelog.md b/docs/project/changelog.md
index b1c66be24..47bf965e5 100644
--- a/docs/project/changelog.md
+++ b/docs/project/changelog.md
@@ -12,6 +12,11 @@ substitutions:
 
 ## [Unreleased]
 
+- {{ API }} {any}`loadPyodide` no longer automatically stores the API into a
+  global variable called `pyodide`. To get old behavior, say `globalThis.pyodide
+  = await loadPyodide({...})`.
+  {pr}`1597`
+
 ## Standard library
 
 - The following standard library modules are now available as standalone packages
diff --git a/docs/usage/loading-packages.md b/docs/usage/loading-packages.md
index eb02c6661..d77381715 100644
--- a/docs/usage/loading-packages.md
+++ b/docs/usage/loading-packages.md
@@ -48,8 +48,9 @@ pyodide.loadPackage(['cycler', 'pytz']);
 {any}`pyodide.loadPackage` returns a `Promise` which resolves when all of the
 packages are finished loading:
 ```javascript
+let pyodide;
 async function main(){
-  await loadPyodide({ 'indexURL' : '<some-url>' });
+  pyodide = await loadPyodide({ 'indexURL' : '<some-url>' });
   await pyodide.loadPackage('matplotlib');
   // matplotlib is now available
 }
@@ -118,9 +119,10 @@ installs from PyPi.
   <script type="text/javascript" src="https://cdn.jsdelivr.net/pyodide/dev/full/pyodide.js"></script>
   <script type="text/javascript">
     async function main(){
-      await loadPyodide({ indexURL : 'https://cdn.jsdelivr.net/pyodide/dev/full/' });
+      let pyodide = await loadPyodide({ indexURL : 'https://cdn.jsdelivr.net/pyodide/dev/full/' });
+      await pyodide.loadPackage("micropip");
       await pyodide.runPythonAsync(`
-        import micropip # runPythonAsync will load micropip automatically
+        import micropip
         await micropip.install('snowballstemmer')
         import snowballstemmer
         stemmer = snowballstemmer.stemmer('english')
diff --git a/docs/usage/quickstart.md b/docs/usage/quickstart.md
index 22545c69a..10fd06023 100644
--- a/docs/usage/quickstart.md
+++ b/docs/usage/quickstart.md
@@ -17,14 +17,13 @@ You can also download a release from [Github
 releases](https://github.com/pyodide/pyodide/releases) or build Pyodide
 yourself. See {ref}`serving_pyodide_packages` for more details.
 
-The `pyodide.js` file defines a single async function called
-{any}`loadPyodide <globalThis.loadPyodide>` which sets up the Python
-environment. When the `loadPyodide` function finishes, Pyodide installs global
-namespace called {js:mod}`pyodide`.
+The `pyodide.js` file defines a single async function called {any}`loadPyodide
+<globalThis.loadPyodide>` which sets up the Python environment and returns {js:mod}`the
+Pyodide top level namespace <pyodide>`.
 
 ```pyodide
 async function main() {
-  await loadPyodide({ indexURL : "https://cdn.jsdelivr.net/pyodide/dev/full/" });
+  let pyodide = await loadPyodide({ indexURL : "https://cdn.jsdelivr.net/pyodide/dev/full/" });
   // Pyodide is now ready to use...
   console.log(pyodide.runPython(`
     import sys
@@ -65,7 +64,7 @@ Create and save a test `index.html` page with the following contents:
     Open your browser console to see Pyodide output
     <script type="text/javascript">
       async function main(){
-        await loadPyodide({
+        let pyodide = await loadPyodide({
           indexURL : "https://cdn.jsdelivr.net/pyodide/dev/full/"
         });
         console.log(pyodide.runPython(`
@@ -112,13 +111,14 @@ Create and save a test `index.html` page with the following contents:
     output.value = 'Initializing...\n';
     // init Pyodide
     async function main(){
-      await loadPyodide({ indexURL : 'https://cdn.jsdelivr.net/pyodide/dev/full/' });
+      let pyodide = await loadPyodide({ indexURL : 'https://cdn.jsdelivr.net/pyodide/dev/full/' });
       output.value += 'Ready!\n';
+      return pyodide;
     }
     let pyodideReadyPromise = main();
 
     async function evaluatePython() {
-      await pyodideReadyPromise;
+      let pyodide = await pyodideReadyPromise;
       try {
         let output = pyodide.runPython(code.value);
         addToOutput(output);
diff --git a/docs/usage/webworker.md b/docs/usage/webworker.md
index 6e3e622e2..058cf05e6 100644
--- a/docs/usage/webworker.md
+++ b/docs/usage/webworker.md
@@ -106,7 +106,7 @@ lines `pythonLoading = self.pyodide.loadPackage(['numpy', 'pytz'])` and
 importScripts('https://cdn.jsdelivr.net/pyodide/dev/full/pyodide.js');
 
 async function loadPyodideAndPackages(){
-    await loadPyodide({ indexURL : 'https://cdn.jsdelivr.net/pyodide/dev/full/' });
+    self.pyodide = await loadPyodide({ indexURL : 'https://cdn.jsdelivr.net/pyodide/dev/full/' });
     await self.pyodide.loadPackage(['numpy', 'pytz']);
 }
 let pyodideReadyPromise = loadPyodideAndPackages();
diff --git a/pyodide-build/pyodide_build/buildpkg.py b/pyodide-build/pyodide_build/buildpkg.py
index a0566efa4..4479c996f 100755
--- a/pyodide-build/pyodide_build/buildpkg.py
+++ b/pyodide-build/pyodide_build/buildpkg.py
@@ -195,7 +195,7 @@ def package_files(buildpath: Path, srcpath: Path, pkg: Dict[str, Any], args):
             "--preload",
             "{}@/".format(install_prefix),
             "--js-output={}".format(name + ".js"),
-            "--export-name=pyodide._module",
+            "--export-name=__pyodide_module",
             "--exclude",
             "*.wasm.pre",
             "--exclude",
diff --git a/src/js/load-pyodide.js b/src/js/load-pyodide.js
index 69d7876a1..d50710a5f 100644
--- a/src/js/load-pyodide.js
+++ b/src/js/load-pyodide.js
@@ -149,8 +149,8 @@ async function _loadPackage(names, messageCallback, errorCallback) {
     let scriptSrc = uri === DEFAULT_CHANNEL ? `${baseURL}${pkg}.js` : uri;
     messageCallback(`Loading ${pkg} from ${scriptSrc}`);
     scriptPromises.push(
-      loadScript(scriptSrc).catch(() => {
-        errorCallback(`Couldn't load package from URL ${scriptSrc}`);
+      loadScript(scriptSrc).catch((e) => {
+        errorCallback(`Couldn't load package from URL ${scriptSrc}`, e);
         toLoad.delete(pkg);
       })
     );
diff --git a/src/js/pyodide.js b/src/js/pyodide.js
index e4febaf16..54ccb0f64 100644
--- a/src/js/pyodide.js
+++ b/src/js/pyodide.js
@@ -143,27 +143,26 @@ function fixRecursionLimit() {
 }
 
 /**
- * The :ref:`js-api-pyodide` module object. Must be present as a global variable
- * called ``pyodide`` in order for package loading to work properly.
- */
-export let pyodide = makePublicAPI();
-
-/**
- * Load the main Pyodide wasm module and initialize it. When finished stores the
- * Pyodide module as a global object called ``pyodide``.
+ * Load the main Pyodide wasm module and initialize it.
+ *
+ * Only one copy of Pyodide can be loaded in a given Javascript global scope
+ * because Pyodide uses global variables to load packages. If an attempt is made
+ * to load a second copy of Pyodide, :any:`loadPyodide` will throw an error.
+ * (This can be fixed once `Firefox adopts support for ES6 modules in webworkers
+ * <https://bugzilla.mozilla.org/show_bug.cgi?id=1247687>`_.)
+ *
  * @param {{ indexURL : string, fullStdLib? : boolean = true }} config
  * @param {string} config.indexURL - The URL from which Pyodide will load
  * packages
  * @param {boolean} config.fullStdLib - Load the full Python standard library.
- * Setting this to false excludes following modules: distutils.
- * Default: true
- * @returns The Pyodide module.
+ * Setting this to false excludes following modules: distutils. Default: true
+ * @returns The :ref:`js-api-pyodide` module.
  * @async
  */
 export async function loadPyodide(config) {
   const default_config = { fullStdLib: true };
   config = Object.assign(default_config, config);
-  if (loadPyodide.inProgress) {
+  if (globalThis.__pyodide_module) {
     if (globalThis.languagePluginURL) {
       throw new Error(
         "Pyodide is already loading because languagePluginURL is defined."
@@ -172,6 +171,9 @@ export async function loadPyodide(config) {
       throw new Error("Pyodide is already loading.");
     }
   }
+  // A global "mount point" for the package loaders to talk to pyodide
+  // See "--export-name=__pyodide_module" in buildpkg.py
+  globalThis.__pyodide_module = Module;
   loadPyodide.inProgress = true;
   // Note: PYODIDE_BASE_URL is an environment variable replaced in
   // in this template in the Makefile. It's recommended to always set
@@ -189,11 +191,9 @@ export async function loadPyodide(config) {
   let packageIndexReady = initializePackageIndex(baseURL);
 
   Module.locateFile = (path) => baseURL + path;
-
   let moduleLoaded = new Promise((r) => (Module.postRun = r));
 
   const scriptSrc = `${baseURL}pyodide.asm.js`;
-
   await loadScript(scriptSrc);
 
   // _createPyodideModule is specified in the Makefile by the linker flag:
@@ -234,6 +234,7 @@ def temp(Module):
   Module.globals = Module.wrapNamespace(Module.globals);
 
   fixRecursionLimit(Module);
+  let pyodide = makePublicAPI();
   pyodide.globals = Module.globals;
   pyodide.pyodide_py = Module.pyodide_py;
   pyodide.version = Module.version;
@@ -241,8 +242,6 @@ def temp(Module):
   registerJsModule("js", globalThis);
   registerJsModule("pyodide_js", pyodide);
 
-  globalThis.pyodide = pyodide;
-
   await packageIndexReady;
   if (config.fullStdLib) {
     await loadPackage(["distutils"]);
@@ -281,5 +280,5 @@ if (globalThis.languagePluginUrl) {
    */
   globalThis.languagePluginLoader = loadPyodide({
     indexURL: globalThis.languagePluginUrl,
-  });
+  }).then((pyodide) => (self.pyodide = pyodide));
 }
diff --git a/src/templates/console.html b/src/templates/console.html
index a22874d95..7c60dbe59 100644
--- a/src/templates/console.html
+++ b/src/templates/console.html
@@ -16,7 +16,7 @@
         return new Promise(resolve => setTimeout(resolve, s));
       }
       async function main() {
-        await loadPyodide({ indexURL : '{{ PYODIDE_BASE_URL }}' });
+        globalThis.pyodide = await loadPyodide({ indexURL : '{{ PYODIDE_BASE_URL }}' });
         let namespace = pyodide.globals.get("dict")();
         pyodide.runPython(`
             import sys
diff --git a/src/webworker.js b/src/webworker.js
index e1a424478..bed6d77f1 100644
--- a/src/webworker.js
+++ b/src/webworker.js
@@ -12,7 +12,7 @@ onmessage = async function (e) {
     }
 
     if (!loadPyodide.inProgress) {
-      await loadPyodide({ indexURL: "{{ PYODIDE_BASE_URL }}" });
+      self.pyodide = await loadPyodide({ indexURL: "{{ PYODIDE_BASE_URL }}" });
     }
     await self.pyodide.loadPackagesFromImports(data.python);
     let results = await self.pyodide.runPythonAsync(data.python);