kill stores.runtimeLoaded and many other stores#850
Merged
Conversation
…nfused with .evaluate(). Instead, introduce a function with a similar behavior in Runtime.runButDontRaise, which is ugly as well and should be killed, but that's for another refactoring
…ing the PyButton class at startup, we create it only after we have the runtime. Note: the old code should be indented, but I didn't do it in this commit to minimize the diff
…since now the class definitions are enclosed inside a function
… sure that the runtime is ready when we execute this code
…e_PyRepl function in the same style as make_PyButton & co. The actual code of PyRepl was not touched at all, this commit just re-indents it
…the places which needs the runtime automatically get it only when it's ready 🎉🎉🎉
madhur-tandon
approved these changes
Oct 16, 2022
madhur-tandon
left a comment
Contributor
There was a problem hiding this comment.
LGTM, the suggestion for hide whitespace helped, thanks for that -- learnt something new.
JeffersGlass
pushed a commit
to JeffersGlass/pyscript
that referenced
this pull request
Oct 17, 2022
As the title stays, the main goal of the branch is to kill the infamous runtimeLoaded global store and all the complications, problems and bugs caused by the fact that in many places we needed to ensure/wait that the global runtime was properly set before being able to execute code. The core idea is that runtime is never a global object and that it's passed around explicitly, which means that when a function receives it, it is guaranteed to be initialized&ready. This caused a bit of complications in pybutton.ts, pyinputbox.ts and pyrepl.ts, because they indirectly want to call runtime.run from connectedCallback, which is the only place where we cannot explicitly pass the runtime because it's automatically called by the browser. But also, it is also a sign of a bad design, because it were entirely possible that connectedCallback was called before the runtime was ready, which probably caused many bugs, see e.g. pyscript#673 and pyscript#747. The solution to is use dependency injection and create the class later on: so instead of having a global PyButton class which relies on a global runtime (whose state is uncertain) we have a make_PyButton function which takes a runtime and make a PyButton class which is tied to that specific runtime (whose state is certainly ready, because we call make_PyButton only when we know that the runtime is ready). Similar for PyInputBox and PyRepl. Other highlights: thanks to this, I could kill the also infamous runAfterRuntimeInitialized and a couple of smelly lines which used setTimeout to "wait" for the runtime. While I was at it, I also called a lot of other stores which were completely unused and where probably leftovers from a past universe.
JeffersGlass
pushed a commit
to JeffersGlass/pyscript
that referenced
this pull request
Oct 31, 2022
As the title stays, the main goal of the branch is to kill the infamous runtimeLoaded global store and all the complications, problems and bugs caused by the fact that in many places we needed to ensure/wait that the global runtime was properly set before being able to execute code. The core idea is that runtime is never a global object and that it's passed around explicitly, which means that when a function receives it, it is guaranteed to be initialized&ready. This caused a bit of complications in pybutton.ts, pyinputbox.ts and pyrepl.ts, because they indirectly want to call runtime.run from connectedCallback, which is the only place where we cannot explicitly pass the runtime because it's automatically called by the browser. But also, it is also a sign of a bad design, because it were entirely possible that connectedCallback was called before the runtime was ready, which probably caused many bugs, see e.g. pyscript#673 and pyscript#747. The solution to is use dependency injection and create the class later on: so instead of having a global PyButton class which relies on a global runtime (whose state is uncertain) we have a make_PyButton function which takes a runtime and make a PyButton class which is tied to that specific runtime (whose state is certainly ready, because we call make_PyButton only when we know that the runtime is ready). Similar for PyInputBox and PyRepl. Other highlights: thanks to this, I could kill the also infamous runAfterRuntimeInitialized and a couple of smelly lines which used setTimeout to "wait" for the runtime. While I was at it, I also called a lot of other stores which were completely unused and where probably leftovers from a past universe.
JeffersGlass
pushed a commit
to JeffersGlass/pyscript
that referenced
this pull request
Nov 14, 2022
As the title stays, the main goal of the branch is to kill the infamous runtimeLoaded global store and all the complications, problems and bugs caused by the fact that in many places we needed to ensure/wait that the global runtime was properly set before being able to execute code. The core idea is that runtime is never a global object and that it's passed around explicitly, which means that when a function receives it, it is guaranteed to be initialized&ready. This caused a bit of complications in pybutton.ts, pyinputbox.ts and pyrepl.ts, because they indirectly want to call runtime.run from connectedCallback, which is the only place where we cannot explicitly pass the runtime because it's automatically called by the browser. But also, it is also a sign of a bad design, because it were entirely possible that connectedCallback was called before the runtime was ready, which probably caused many bugs, see e.g. pyscript#673 and pyscript#747. The solution to is use dependency injection and create the class later on: so instead of having a global PyButton class which relies on a global runtime (whose state is uncertain) we have a make_PyButton function which takes a runtime and make a PyButton class which is tied to that specific runtime (whose state is certainly ready, because we call make_PyButton only when we know that the runtime is ready). Similar for PyInputBox and PyRepl. Other highlights: thanks to this, I could kill the also infamous runAfterRuntimeInitialized and a couple of smelly lines which used setTimeout to "wait" for the runtime. While I was at it, I also called a lot of other stores which were completely unused and where probably leftovers from a past universe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As the title stays, the main goal of the branch is to kill the infamous
runtimeLoadedglobal store and all the complications, problems and bugs caused by the fact that in many places we needed to ensure/wait that the global runtime was properly set before being able to execute code.The core idea is that
runtimeis never a global object and that it's passed around explicitly, which means that when a function receives it, it is guaranteed to be initialized&ready.This caused a bit of complications in
pybutton.ts,pyinputbox.tsandpyrepl.ts, because they indirectly want to callruntime.runfromconnectedCallback, which is the only place where we cannot explicitly pass theruntimebecause it's automatically called by the browser.But also, it is also a sign of a bad design, because it were entirely possible that
connectedCallbackwas called before the runtime was ready, which probably caused many bugs, see e.g. #673 and #747.The solution to is use dependency injection and create the class later on: so instead of having a global
PyButtonclass which relies on a globalruntime(whose state is uncertain) we have amake_PyButtonfunction which takes a runtime and make aPyButtonclass which is tied to that specific runtime (whose state is certainly ready, because we callmake_PyButtononly when we know that the runtime is ready).Similar for
PyInputBoxandPyRepl.Other highlights: thanks to this, I could kill the also infamous
runAfterRuntimeInitializedand a couple of smelly lines which usedsetTimeoutto "wait" for the runtime.While I was at it, I also called a lot of other stores which were completely unused and where probably leftovers from a past universe.
Note for reviewers: the bare diff is bigger than it should, because the majority of code in
pyinputbox.ts,pybutton.tsandpyrepl.tswas indented by one level, without actually touching it. In order to minimize the diff and concentrate to the actual changes, I suggest to either review commits one by one (they are all nicely self-contained) or to use the "hide whitespace" option in the "Files changed" tab.