Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequentially run all the requests in the script file #40

Merged
merged 5 commits into from Feb 13, 2020
Merged

Conversation

@dylanowen
Copy link
Contributor

dylanowen commented Feb 3, 2020

#10

This is my first pass at adding the ability to run all the requests in a file sequentially. You can enable it via the command line flag -a

I assumed that we didn't want to allow persisting global state except inside of the global object. To facilitate this I chose the initialize function (src/script_engine/boa.rs:115) as the point where the request engine would reset it's internal state for the next request.

Right now my implementation isn't the most efficient, I'm de/serializing everything out to strings between each request, also on the first request I recreate an already empty js Realm.

Let me know if you have any suggestions or if this doesn't fit into how you imagined structuring the project.

-Thanks

Copy link
Owner

bayne left a comment

Thanks for the contribution!

This all looks great and I appreciate the initial description. I removed the branch name linter so if you were to rebase on master we should get all green 🤞.

I intended initialize to be something that is called at the start of the program. ScriptEngine was meant to be an abstraction over the underlying implementation of the javascript engine (one day I might want to replace Boa with something else).

Instead of updating initialize let's add a new public function to the ScriptEngine trait for resetting the global state. Then we could use this function (instead of initialize) to make sure that the next response handler doesn't carry state from the previous response handler.

dylanowen added 2 commits Feb 13, 2020
@dylanowen

This comment has been minimized.

Copy link
Contributor Author

dylanowen commented Feb 13, 2020

I might have got carried away with the changes to make this possible.

I changed initialize to store the initialization state so we can run the reset function each time. To deal with the borrow checker it made it easier to have Interpreter implement a simple parse/execute trait so we can disjointly interact with the im/mutable portions of our BoaScriptEngine. I also changed a lot of the ScriptEngine functions to not take in a mutable reference and to take in references to the Scripts instead. This made it easier to reuse our scripts.

@bayne
bayne approved these changes Feb 13, 2020
@bayne bayne merged commit 81192a3 into bayne:master Feb 13, 2020
3 checks passed
3 checks passed
Check
Details
Test Suite
Details
Lints
Details
@dylanowen dylanowen deleted the dylanowen:all branch Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.