Make the life-cycle more linear and kill some svelte stores by antocuni · Pull Request #830 · pyscript/pyscript

antocuni · 2022-10-06T11:37:16Z

This is a follow-up of PR #806 and it's a big step forward towards solving issue #763.

The basic idea is that at this stage I want to streamline the execution logic and make it as linear and easy to read/understand as possible. Once we have a better understanding of what's going on, we can think of slicing it again in forms of plugins or similar, if we want.

The diff is relatively big, but for the most part is just "shuffling code around". The biggest outcome is that now the majority of the execution logic of the page follows a very linear logic, which I describe in a comment at the beginning of main.ts:

pyscript/pyscriptjs/src/main.ts

Lines 27 to 56 in 42d81db

    
           /* High-level overview of the lifecycle of a PyScript App: 
        
              1. pyscript.js is loaded by the browser. PyScriptApp().main() is called 
        
              2. loadConfig(): search for py-config and compute the config for the app 
        
              3. show the loader/splashscreen 
        
              4. loadRuntime(): start downloading the actual runtime (e.g. pyodide.js) 
        
              --- wait until (4) has finished --- 
        
              5. now the pyodide src is available. Initialize the engine 
        
              6. setup the environment, install packages 
        
              7. run user scripts 
        
              8. initialize the rest of web components such as py-button, py-repl, etc. 
        
           More concretely: 
        
             - Points 1-4 are implemented sequentially in PyScriptApp.main(). 
        
             - PyScriptApp.loadRuntime adds a <script> tag to the document to initiate 
        
               the download, and then adds an event listener for the 'load' event, which 
        
               in turns calls PyScriptApp.afterRuntimeLoad(). 
        
             - PyScriptApp.afterRuntimeLoad() implements all the points >= 5. 
        
           */

Previously, a big chunk of the logic was inside runtime.ts: the old Runtime.initialize is now PyScriptApp.afterRuntimeLoad, with some small refactoring to split the code. Also, the order of functions inside main.ts follows the execution order, so that you can nicely read it from top to bottom:

pyscript/pyscriptjs/src/main.ts

Lines 128 to 166 in 42d81db

    
           // lifecycle (5) 
        
           // See the overview comment above for an explanation of how we jump from 
        
           // point (4) to point (5). 
        
           // 
        
           // Invariant: this.config and this.loader are set and available. 
        
           async afterRuntimeLoad(runtime: Runtime): Promise<void> { 
        
               // XXX what is the JS/TS standard way of doing asserts? 
        
               console.assert(this.config !== undefined); 
        
               console.assert(this.loader !== undefined); 
        
               this.loader.log('Python startup...'); 
        
               await runtime.loadInterpreter(); 
        
               runtimeLoaded.set(runtime); 
        
               this.loader.log('Python ready!'); 
        
               // eslint-disable-next-line 
        
               runtime.globals.set('pyscript_loader', this.loader); 
        
               this.loader.log('Setting up virtual environment...'); 
        
               await this.setupVirtualEnv(runtime); 
        
               await mountElements(runtime); 
        
               this.loader.log('Executing <py-script> tags...'); 
        
               this.executeScripts(runtime); 
        
               this.loader.log('Initializing web components...'); 
        
               // lifecycle (8) 
        
               createCustomElements(); 
        
               if (runtime.config.autoclose_loader) { 
        
                   this.loader.close(); 
        
               } 
        
               await initHandlers(runtime); 
        
               // NOTE: runtime message is used by integration tests to know that 
        
               // pyscript initialization has complete. If you change it, you need to 
        
               // change it also in tests/integration/support.py 
        
               logger.info('PyScript page fully initialized'); 
        
           }

Svelte stores
The idea is to eventually kill of them, so that we can remove the dependency on svelte but also to avoid relying on so much global state, which makes things more complicated and error-prone (e.g., we have several issues related to using runtime when it's not ready yet).

I killed addInitializer, addPostInitializer and the corresponding svelte stores 🎉. They are no longer needed since the relevant code is called directly from main.ts.

I started to kill the usage of the runtimeLoaded svelte store: instead of relying on a global variable, I want to arrive at the point in which the runtime is passed as a parameter in all places where it's needed: pyscript.ts is now free of global state, but I couldn't kill it yet because it's used heavily by base.ts and pyrepl.ts. I will do it in another PR.

Other misc changes
I added sanity checks (and corresponding tests!) which complain if you specify 0 or multiple runtimes. Currently we support having one and only one, so there is no point to pretend otherwise

I modified the messages displayed by the loader, to be more informative from the user point of view. Now they look like this:

…an error if we specify 0 or >1

…runtime.ts to main.ts, document what is the expected lifecycle and put comments around to guide the reader around the code. Many more improvements are possible, but this tries to be as close as possible to the old logic

…orresponds to the order of execution

…xplicitly

marimeireles

This looks great!
So much more organized and easier to understand. Great work @antocuni :)

marimeireles · 2022-10-07T13:07:34Z

+scriptsQueue.subscribe((value: PyScript[]) => {
+    scriptsQueue_ = value;


I wonder if it's worth it to change these names? I think these they are so confusing... Do you have an idea for better naming?

they are going to be killed soon ☠️

marimeireles · 2022-10-07T13:08:01Z

+
+/* High-level overview of the lifecycle of a PyScript App:
+
+   1. pyscript.js is loaded by the browser. PyScriptApp().main() is called
+
+   2. loadConfig(): search for py-config and compute the config for the app
+
+   3. show the loader/splashscreen
+
+   4. loadRuntime(): start downloading the actual runtime (e.g. pyodide.js)
+
+   --- wait until (4) has finished ---
+
+   5. now the pyodide src is available. Initialize the engine
+
+   6. setup the environment, install packages
+
+   7. run user scripts
+
+   8. initialize the rest of web components such as py-button, py-repl, etc.
+
+More concretely:
+
+  - Points 1-4 are implemented sequentially in PyScriptApp.main().
+
+  - PyScriptApp.loadRuntime adds a <script> tag to the document to initiate
+    the download, and then adds an event listener for the 'load' event, which
+    in turns calls PyScriptApp.afterRuntimeLoad().
+
+  - PyScriptApp.afterRuntimeLoad() implements all the points >= 5.
+*/
+
+


Grateful for docs in the code 🙏

marimeireles · 2022-10-07T13:27:53Z

+        script.addEventListener('load', () => {
+            void this.afterRuntimeLoad(runtime);


Is this just the way you do lambdas in TS?
(Asking because I'm TS noob)

yes, it's not even TS-specific, it works the same in plain JS. They are not called lambda though, they are just "functions" and/or "closures".
This particular syntax is called "arrow function", which of course has slightly different rules than normal functions 🤦‍♂️

DISCLAIMER: I'm not the author of this code, I just moved it around :)

antocuni added 10 commits October 6, 2022 13:36

kill loadedEnvironments & co. It seems they are not actually used

58fb4e8

kill also this

e6a7eed

don't try to pretend that we can support multiple runtimes, and show …

138b2ce

…an error if we specify 0 or >1

woooo, kill initiailizers and postInitializers 🎉

0bc3618

kill the global runtime variable and pass the runtime explicitly

d3e77cd

just shuffle code around so that the order of functions in the file c…

e13dd38

…orresponds to the order of execution

kill the last occurrence of runtimeSpec, always pass runtime around e…

647c750

…xplicitly

use better user-facing messages during startup

42d81db

this is not needed

5bb3309

antocuni changed the title ~~WIP: more life-cycle refactoring~~ Make the life-cycle more linear and kill some svelte stores Oct 7, 2022

antocuni marked this pull request as ready for review October 7, 2022 08:01

tedpatrick reviewed Oct 7, 2022

View reviewed changes

Comment thread pyscriptjs/src/main.ts

tedpatrick reviewed Oct 7, 2022

View reviewed changes

Comment thread pyscriptjs/src/main.ts

tedpatrick reviewed Oct 7, 2022

View reviewed changes

Comment thread pyscriptjs/src/main.ts

tedpatrick approved these changes Oct 7, 2022

View reviewed changes

marimeireles approved these changes Oct 7, 2022

View reviewed changes

antocuni merged commit 11a517b into main Oct 7, 2022

antocuni deleted the antocuni/refactor-life-cycle branch October 7, 2022 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the life-cycle more linear and kill some svelte stores#830

Make the life-cycle more linear and kill some svelte stores#830
antocuni merged 10 commits into
mainfrom
antocuni/refactor-life-cycle

antocuni commented Oct 6, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marimeireles left a comment

Uh oh!

marimeireles Oct 7, 2022

Uh oh!

antocuni Oct 7, 2022

Uh oh!

marimeireles Oct 7, 2022

Uh oh!

marimeireles Oct 7, 2022

Uh oh!

antocuni Oct 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	/* High-level overview of the lifecycle of a PyScript App:

	1. pyscript.js is loaded by the browser. PyScriptApp().main() is called

	2. loadConfig(): search for py-config and compute the config for the app

	3. show the loader/splashscreen

	4. loadRuntime(): start downloading the actual runtime (e.g. pyodide.js)

	--- wait until (4) has finished ---

	5. now the pyodide src is available. Initialize the engine

	6. setup the environment, install packages

	7. run user scripts

	8. initialize the rest of web components such as py-button, py-repl, etc.

	More concretely:

	- Points 1-4 are implemented sequentially in PyScriptApp.main().

	- PyScriptApp.loadRuntime adds a <script> tag to the document to initiate
	the download, and then adds an event listener for the 'load' event, which
	in turns calls PyScriptApp.afterRuntimeLoad().

	- PyScriptApp.afterRuntimeLoad() implements all the points >= 5.
	*/

	// lifecycle (5)
	// See the overview comment above for an explanation of how we jump from
	// point (4) to point (5).
	//
	// Invariant: this.config and this.loader are set and available.
	async afterRuntimeLoad(runtime: Runtime): Promise<void> {
	// XXX what is the JS/TS standard way of doing asserts?
	console.assert(this.config !== undefined);
	console.assert(this.loader !== undefined);

	this.loader.log('Python startup...');
	await runtime.loadInterpreter();
	runtimeLoaded.set(runtime);
	this.loader.log('Python ready!');

	// eslint-disable-next-line
	runtime.globals.set('pyscript_loader', this.loader);

	this.loader.log('Setting up virtual environment...');
	await this.setupVirtualEnv(runtime);
	await mountElements(runtime);

	this.loader.log('Executing <py-script> tags...');
	this.executeScripts(runtime);

	this.loader.log('Initializing web components...');
	// lifecycle (8)
	createCustomElements();

	if (runtime.config.autoclose_loader) {
	this.loader.close();
	}
	await initHandlers(runtime);

	// NOTE: runtime message is used by integration tests to know that
	// pyscript initialization has complete. If you change it, you need to
	// change it also in tests/integration/support.py
	logger.info('PyScript page fully initialized');
	}

		scriptsQueue.subscribe((value: PyScript[]) => {
		scriptsQueue_ = value;

		script.addEventListener('load', () => {
		void this.afterRuntimeLoad(runtime);

Conversation

antocuni commented Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marimeireles left a comment

Choose a reason for hiding this comment

Uh oh!

marimeireles Oct 7, 2022

Choose a reason for hiding this comment

Uh oh!

antocuni Oct 7, 2022

Choose a reason for hiding this comment

Uh oh!

marimeireles Oct 7, 2022

Choose a reason for hiding this comment

Uh oh!

marimeireles Oct 7, 2022

Choose a reason for hiding this comment

Uh oh!

antocuni Oct 7, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

antocuni commented Oct 6, 2022 •

edited

Loading