implement proposal for fetching paths and retaining structure of dirs and packages (#914)

* implement proposal

* update docs and replace py-env

* more docs

* suggested proposal

* update docs

* add to_file parameter

* remove comment from Makefile

* suggested improvements

* move tests from basic to py_config

* retain leading slash from the first path

pyscriptjs/Makefile

@@ -95,7 +95,6 @@ test-py:
 	$(PYTEST_EXE) -vv $(ARGS) tests/py-unit/ --log-cli-level=warning
 
 test-ts:
-	@echo "Tests are coming :( this is a placeholder and it's meant to fail!"
 	npm run test
 
 fmt: fmt-py fmt-ts

pyscriptjs/src/main.ts

@@ -8,6 +8,7 @@ import { PyLoader } from './components/pyloader';
 import { PyodideRuntime } from './pyodide';
 import { getLogger } from './logger';
 import { handleFetchError, showError, globalExport } from './utils';
+import { calculatePaths } from './plugins/fetch';
 import { createCustomElements } from './components/elements';
 
 type ImportType = { [key: string]: unknown };
@@ -171,15 +172,15 @@ class PyScriptApp {
         // it in Python, which means we need to have the runtime
         // initialized. But we could easily do it in JS in parallel with the
         // download/startup of pyodide.
-        const paths = this.config.paths;
-        logger.info('Paths to fetch: ', paths);
-        for (const singleFile of paths) {
-            logger.info(`  fetching path: ${singleFile}`);
+        const [paths, fetchPaths] = calculatePaths(this.config.fetch);
+        logger.info('Paths to fetch: ', fetchPaths);
+        for (let i=0; i<paths.length; i++) {
+            logger.info(`  fetching path: ${fetchPaths[i]}`);
             try {
-                await runtime.loadFromFile(singleFile);
+                await runtime.loadFromFile(paths[i], fetchPaths[i]);
             } catch (e) {
                 //Should we still export full error contents to console?
-                handleFetchError(<Error>e, singleFile);
+                handleFetchError(<Error>e, fetchPaths[i]);
             }
         }
         logger.info('All paths fetched');

pyscriptjs/src/plugins/fetch.ts

@@ -0,0 +1,39 @@
+import { joinPaths } from '../utils';
+import { FetchConfig } from "../pyconfig";
+
+export function calculatePaths(fetch_cfg: FetchConfig[]) {
+    const fetchPaths: string[] = [];
+    const paths: string[] = [];
+    fetch_cfg.forEach(function (each_fetch_cfg: FetchConfig) {
+        const from = each_fetch_cfg.from || "";
+        const to_folder = each_fetch_cfg.to_folder || ".";
+        const to_file = each_fetch_cfg.to_file;
+        const files = each_fetch_cfg.files;
+        if (files !== undefined)
+        {
+            if (to_file !== undefined)
+            {
+                throw Error(`Cannot use 'to_file' and 'files' parameters together!`);
+            }
+            for (const each_f of files)
+            {
+                const each_fetch_path = joinPaths([from, each_f]);
+                fetchPaths.push(each_fetch_path);
+                const each_path = joinPaths([to_folder, each_f]);
+                paths.push(each_path);
+            }
+        }
+        else
+        {
+            fetchPaths.push(from);
+            const filename = to_file || from.split('/').pop();
+            if (filename === '') {
+                throw Error(`Couldn't determine the filename from the path ${from}, supply ${to_file} parameter!`);
+            }
+            else {
+                paths.push(joinPaths([to_folder, filename]));
+            }
+        }
+    });
+    return [paths, fetchPaths];
+}

pyscriptjs/src/pyconfig.ts

@@ -17,11 +17,18 @@ export interface AppConfig extends Record<string, any> {
     autoclose_loader?: boolean;
     runtimes?: RuntimeConfig[];
     packages?: string[];
-    paths?: string[];
+    fetch?: FetchConfig[];
     plugins?: string[];
     pyscript?: PyScriptMetadata;
 }
 
+export type FetchConfig = {
+    from?: string;
+    to_folder?: string;
+    to_file?: string;
+    files?: string[];
+};
+
 export type RuntimeConfig = {
     src?: string;
     name?: string;
@@ -37,7 +44,7 @@ const allKeys = {
     string: ['name', 'description', 'version', 'type', 'author_name', 'author_email', 'license'],
     number: ['schema_version'],
     boolean: ['autoclose_loader'],
-    array: ['runtimes', 'packages', 'paths', 'plugins'],
+    array: ['runtimes', 'packages', 'fetch', 'plugins'],
 };
 
 export const defaultConfig: AppConfig = {
@@ -52,7 +59,7 @@ export const defaultConfig: AppConfig = {
         },
     ],
     packages: [],
-    paths: [],
+    fetch: [],
     plugins: [],
 };
 
@@ -183,9 +190,9 @@ function validateConfig(configText: string, configType = 'toml') {
             if (validateParamInConfig(item, keyType, config)) {
                 if (item === 'runtimes') {
                     finalConfig[item] = [];
-                    const runtimes = config[item] as object[];
-                    runtimes.forEach(function (eachRuntime: object) {
-                        const runtimeConfig: object = {};
+                    const runtimes = config[item] as RuntimeConfig[];
+                    runtimes.forEach(function (eachRuntime: RuntimeConfig) {
+                        const runtimeConfig: RuntimeConfig = {};
                         for (const eachRuntimeParam in eachRuntime) {
                             if (validateParamInConfig(eachRuntimeParam, 'string', eachRuntime)) {
                                 runtimeConfig[eachRuntimeParam] = eachRuntime[eachRuntimeParam];
@@ -193,7 +200,22 @@ function validateConfig(configText: string, configType = 'toml') {
                         }
                         finalConfig[item].push(runtimeConfig);
                     });
-                } else {
+                }
+                else if (item === 'fetch') {
+                    finalConfig[item] = [];
+                    const fetchList = config[item] as FetchConfig[];
+                    fetchList.forEach(function (eachFetch: FetchConfig) {
+                        const eachFetchConfig: FetchConfig = {};
+                        for (const eachFetchConfigParam in eachFetch) {
+                            const targetType = eachFetchConfigParam === 'files' ? 'array' : 'string';
+                            if (validateParamInConfig(eachFetchConfigParam, targetType, eachFetch)) {
+                                eachFetchConfig[eachFetchConfigParam] = eachFetch[eachFetchConfigParam];
+                            }
+                        }
+                        finalConfig[item].push(eachFetchConfig);
+                    });
+                }
+                else {
                     finalConfig[item] = config[item];
                 }
             }

pyscriptjs/src/pyodide.ts

@@ -92,7 +92,7 @@ export class PyodideRuntime extends Runtime {
         }
     }
 
-    async loadFromFile(path: string): Promise<void> {
+    async loadFromFile(path: string, fetch_path: string): Promise<void> {
         const pathArr = path.split('/');
         const filename = pathArr.pop();
         for (let i = 0; i < pathArr.length; i++) {
@@ -105,7 +105,7 @@ export class PyodideRuntime extends Runtime {
                 this.interpreter.FS.mkdir(eachPath);
             }
         }
-        const response = await fetch(path);
+        const response = await fetch(fetch_path);
         const buffer = await response.arrayBuffer();
         const data = new Uint8Array(buffer);
         pathArr.push(filename);

pyscriptjs/src/runtime.ts

@@ -93,5 +93,5 @@ export abstract class Runtime extends Object {
      * delegates the loading of files to the
      * underlying interpreter.
      * */
-    abstract loadFromFile(path: string): Promise<void>;
+    abstract loadFromFile(path: string, fetch_path: string): Promise<void>;
 }

pyscriptjs/src/utils.ts

@@ -109,3 +109,12 @@ export function getAttribute(el: Element, attr: string): string | null {
     }
     return null;
 }
+
+export function joinPaths(parts: string[], separator = '/') {
+    const res = parts.map(function(part) { return part.trim().replace(/(^[/]*|[/]*$)/g, ''); }).filter(p => p!== "").join(separator || '/');
+    if (parts[0].startsWith('/'))
+    {
+        return '/'+res;
+    }
+    return res;
+}

pyscriptjs/tests/integration/test_01_basic.py

@@ -73,71 +73,6 @@ class TestBasic(PyScriptTest):
         )
         assert self.console.log.lines == [self.PY_COMPLETE, "true false", "<div></div>"]
 
-    def test_paths(self):
-        self.writefile("a.py", "x = 'hello from A'")
-        self.writefile("b.py", "x = 'hello from B'")
-        self.pyscript_run(
-            """
-            <py-config>
-                paths = ["./a.py", "./b.py"]
-            </py-config>
-
-            <py-script>
-                import js
-                import a, b
-                js.console.log(a.x)
-                js.console.log(b.x)
-            </py-script>
-            """
-        )
-        assert self.console.log.lines == [
-            self.PY_COMPLETE,
-            "hello from A",
-            "hello from B",
-        ]
-
-    def test_paths_that_do_not_exist(self):
-        self.pyscript_run(
-            """
-            <py-config>
-                paths = ["./f.py"]
-            </py-config>
-            """
-        )
-        assert self.console.error.lines == ["Failed to load resource: net::ERR_FAILED"]
-        assert self.console.warning.lines == [
-            "Caught an error in fetchPaths:\r\n TypeError: Failed to fetch"
-        ]
-
-        errorContent = """PyScript: Access to local files
-        (using "Paths:" in &lt;py-config&gt;)
-        is not available when directly opening a HTML file;
-        you must use a webserver to serve the additional files."""
-
-        inner_html = self.page.locator(".py-error").inner_html()
-        assert errorContent in inner_html
-
-    def test_paths_from_packages(self):
-        self.writefile("utils/__init__.py", "")
-        self.writefile("utils/a.py", "x = 'hello from A'")
-        self.pyscript_run(
-            """
-            <py-config>
-                paths = ["./utils/__init__.py", "./utils/a.py"]
-            </py-config>
-
-            <py-script>
-                import js
-                from utils.a import x
-                js.console.log(x)
-            </py-script>
-            """
-        )
-        assert self.console.log.lines == [
-            self.PY_COMPLETE,
-            "hello from A",
-        ]
-
     def test_packages(self):
         self.pyscript_run(
             """

pyscriptjs/tests/integration/test_py_config.py

@@ -200,3 +200,73 @@ class TestConfig(PyScriptTest):
         )
         assert div.text_content() == expected
         assert self.console.log.lines[-1] == "hello world"
+
+    def test_paths(self):
+        self.writefile("a.py", "x = 'hello from A'")
+        self.writefile("b.py", "x = 'hello from B'")
+        self.pyscript_run(
+            """
+            <py-config>
+                [[fetch]]
+                files = ["./a.py", "./b.py"]
+            </py-config>
+
+            <py-script>
+                import js
+                import a, b
+                js.console.log(a.x)
+                js.console.log(b.x)
+            </py-script>
+            """
+        )
+        assert self.console.log.lines == [
+            self.PY_COMPLETE,
+            "hello from A",
+            "hello from B",
+        ]
+
+    def test_paths_that_do_not_exist(self):
+        self.pyscript_run(
+            """
+            <py-config>
+                [[fetch]]
+                files = ["./f.py"]
+            </py-config>
+            """
+        )
+        assert self.console.error.lines == ["Failed to load resource: net::ERR_FAILED"]
+        assert self.console.warning.lines == [
+            "Caught an error in fetchPaths:\r\n TypeError: Failed to fetch"
+        ]
+
+        errorContent = """PyScript: Access to local files
+        (using "Paths:" in &lt;py-config&gt;)
+        is not available when directly opening a HTML file;
+        you must use a webserver to serve the additional files."""
+
+        inner_html = self.page.locator(".py-error").inner_html()
+        assert errorContent in inner_html
+
+    def test_paths_from_packages(self):
+        self.writefile("utils/__init__.py", "")
+        self.writefile("utils/a.py", "x = 'hello from A'")
+        self.pyscript_run(
+            """
+            <py-config>
+                [[fetch]]
+                from = "utils"
+                to_folder = "pkg"
+                files = ["__init__.py", "a.py"]
+            </py-config>
+
+            <py-script>
+                import js
+                from pkg.a import x
+                js.console.log(x)
+            </py-script>
+            """
+        )
+        assert self.console.log.lines == [
+            self.PY_COMPLETE,
+            "hello from A",
+        ]

pyscriptjs/tests/unit/fakeruntime.ts

@@ -33,7 +33,7 @@ export class FakeRuntime extends Runtime {
         throw new Error("not implemented");
     }
 
-    async loadFromFile(path: string) {
+    async loadFromFile(path: string, fetch_path: string) {
         throw new Error("not implemented");
     }
 }

pyscriptjs/tests/unit/fetch_plugin.test.ts

@@ -0,0 +1,49 @@
+import { calculatePaths } from "../../src/plugins/fetch";
+import { FetchConfig } from "../../src/pyconfig";
+
+describe("CalculateFetchPaths", () => {
+    it("should calculate paths when only from is provided", () => {
+      const fetch_cfg: FetchConfig[] = [{from: "http://a.com/data.csv" }];
+      const [paths, fetchPaths] = calculatePaths(fetch_cfg);
+      expect(paths).toStrictEqual(["./data.csv"]);
+      expect(fetchPaths).toStrictEqual(["http://a.com/data.csv"]);
+    })
+
+    it("should calculate paths when only files is provided", () => {
+      const fetch_cfg: FetchConfig[] = [{files: ["foo/__init__.py", "foo/mod.py"] }];
+      const [paths, fetchPaths] = calculatePaths(fetch_cfg);
+      expect(paths).toStrictEqual(["./foo/__init__.py", "./foo/mod.py"]);
+      expect(fetchPaths).toStrictEqual(["foo/__init__.py", "foo/mod.py"]);
+    })
+
+    it("should calculate paths when files and to_folder is provided", () => {
+      const fetch_cfg: FetchConfig[] = [{files: ["foo/__init__.py", "foo/mod.py"], to_folder: "/my/lib/"}];
+      const [paths, fetchPaths] = calculatePaths(fetch_cfg);
+      expect(paths).toStrictEqual(["/my/lib/foo/__init__.py", "/my/lib/foo/mod.py"]);
+      expect(fetchPaths).toStrictEqual(["foo/__init__.py", "foo/mod.py"]);
+    })
+
+    it("should calculate paths when from and files and to_folder is provided", () => {
+      const fetch_cfg: FetchConfig[] = [{from: "http://a.com/download/", files: ["foo/__init__.py", "foo/mod.py"], to_folder: "/my/lib/"}];
+      const [paths, fetchPaths] = calculatePaths(fetch_cfg);
+      expect(paths).toStrictEqual(["/my/lib/foo/__init__.py", "/my/lib/foo/mod.py"]);
+      expect(fetchPaths).toStrictEqual(["http://a.com/download/foo/__init__.py", "http://a.com/download/foo/mod.py"]);
+    })
+
+    it("should error out while calculating paths when filename cannot be determined from 'from'", () => {
+      const fetch_cfg: FetchConfig[] = [{from: "http://google.com/", to_folder: "/tmp"}];
+      expect(()=>calculatePaths(fetch_cfg)).toThrowError("Couldn't determine the filename from the path http://google.com/");
+    })
+
+    it("should calculate paths when to_file is explicitly supplied", () => {
+      const fetch_cfg: FetchConfig[] = [{from: "http://a.com/data.csv?version=1", to_file: "pkg/tmp/data.csv"}];
+      const [paths, fetchPaths] = calculatePaths(fetch_cfg);
+      expect(paths).toStrictEqual(["./pkg/tmp/data.csv"]);
+      expect(fetchPaths).toStrictEqual(["http://a.com/data.csv?version=1"]);
+    })
+
+    it("should error out when both to_file and files parameters are provided", () => {
+      const fetch_cfg: FetchConfig[] = [{from: "http://a.com/data.csv?version=1", to_file: "pkg/tmp/data.csv", files: ["a.py", "b.py"]}];
+      expect(()=>calculatePaths(fetch_cfg)).toThrowError("Cannot use 'to_file' and 'files' parameters together!");
+    })
+})

pyscriptjs/tests/unit/pybox.test.ts

@@ -1,5 +1,3 @@
-import { jest } from "@jest/globals"
-
 import { PyBox } from "../../src/components/pybox"
 
 customElements.define('py-box', PyBox)

pyscriptjs/tests/unit/pybutton.test.ts

@@ -1,4 +1,3 @@
-import { jest } from '@jest/globals';
 import type { Runtime } from "../../src/runtime"
 import { FakeRuntime } from "./fakeruntime"
 import { make_PyButton } from '../../src/components/pybutton';

pyscriptjs/tests/unit/pyconfig.test.ts

@@ -1,5 +1,4 @@
 import { jest } from '@jest/globals';
-import type { AppConfig, RuntimeConfig } from '../../src/pyconfig';
 import { loadConfigFromElement, defaultConfig } from '../../src/pyconfig';
 import { version } from '../../src/runtime';
 

pyscriptjs/tests/unit/pytitle.test.ts

@@ -1,5 +1,3 @@
-import { jest } from "@jest/globals";
-
 import { PyTitle } from "../../src/components/pytitle"
 
 

pyscriptjs/tests/unit/utils.test.ts

@@ -1,5 +1,5 @@
-import { jest } from "@jest/globals"
-import { ensureUniqueId } from "../../src/utils"
+import { ensureUniqueId, joinPaths } from '../../src/utils';
+import { expect } from "@jest/globals";
 
 describe("Utils", () => {
 
@@ -32,3 +32,23 @@ describe("Utils", () => {
     expect(secondElement.id).toBe("py-internal-2")
   })
 })
+
+describe("JoinPaths", () => {
+  it("should remove trailing slashes from the beginning and the end", () => {
+    const paths: string[] = ['///abc/d/e///'];
+    const joinedPath = joinPaths(paths);
+    expect(joinedPath).toStrictEqual('/abc/d/e');
+  })
+
+  it("should not remove slashes from the middle to preserve protocols such as http", () => {
+    const paths: string[] = ['http://google.com', '///data.txt'];
+    const joinedPath = joinPaths(paths);
+    expect(joinedPath).toStrictEqual('http://google.com/data.txt');
+  })
+
+  it("should not join paths when they are empty strings", () => {
+    const paths: string[] = ['', '///hhh/ll/pp///', '', 'kkk'];
+    const joinedPath = joinPaths(paths);
+    expect(joinedPath).toStrictEqual('hhh/ll/pp/kkk');
+  })
+})