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

Split WebAPI (environment) built-in objects from the ES builtin objects. #718

Closed
croraf opened this issue Sep 26, 2020 · 5 comments · Fixed by #800
Closed

Split WebAPI (environment) built-in objects from the ES builtin objects. #718

croraf opened this issue Sep 26, 2020 · 5 comments · Fixed by #800
Assignees
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request
Milestone

Comments

@croraf
Copy link
Contributor

croraf commented Sep 26, 2020

Split WebAPI (environment) objects from the ES builtin objects.

Perhaps delegate the implementation of environment objects to some other library.

@jasonwilliams
Copy link
Member

I understand that things like console shouldn’t be in the engine, but it is used for debugging purposes and is making lives easier right now.

This will most likely be a low priority issue as I think Console is the only object that is external from the spec, and we benefit more from having it right now than we do by getting rid of it.
I also don’t know if many are embedding Boa right now for it to be a problem

@croraf
Copy link
Contributor Author

croraf commented Sep 26, 2020

OK, but I would at least think of separating the Console from the "/boa" JS engine folder, and would inject it somehow into the environment (for example from the "boa_cli" or something)?

Doing this would also as a prerequisite require an API for putting external builtin objects into the execution context (or whatever is the proper terminology).

@HalidOdat HalidOdat added builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request labels Sep 27, 2020
@HalidOdat
Copy link
Member

HalidOdat commented Sep 27, 2020

Proposed changes:

  • We put our implementation of the console object in an feature flags (maybe console/console-object flag) this will on by default or not, not sure.

  • Later on once we implement Implement an easily replaceable Printer for console #551

    • We will have a builder struct called ContextBuilder with will impl:
impl ContextBuilder {
	#[cfg(feature = "console")]
	pub fn console<T>(&mut self, value: T) -> &mut Self
	where
		T: Into<Console>,
	{
	}

	fn build() -> &mut Self {
		// ...	
	}
}

so we can do something like this:

let mut context = Context::builder()
	.console(MyConsole) // MyConsole implements Console as described in #551
	.build();

// ...

later on we will be able to extend the ContextBuilder struct to maybe disable/enable some builtin objects like Map/Set/Proxy/...

@Razican
Copy link
Member

Razican commented Oct 2, 2020

The way I see it, console is different in boa_wasm and boa_cli, for example. The first should print in the browser while the second one should print in the OS console.

JavaScript has no notion of console or anything, so something we could do here is to use a register hook to add this. We don't have a register_global_variable hook, right? maybe we should, and we could use that to add stuff if needed.

Note that the hook itself is needed for full test262 executor compliance, since we need to add some global variables + functions.

Once we have that, we can move the console implementation to its own crate, something like boa_console, that accepts a printer as described in #551. This would be used in the boa_cli crate, while the boa_wasm could inject its own implementation, I think, right?

@croraf
Copy link
Contributor Author

croraf commented Oct 6, 2020

If we go with "the inject on context creation" route, perhaps the following can be used.


The current flow goes like this:

boa_cli/src/main.rs let mut engine = Context::new();
calls
boa/src/context.rs

pub fn new() -> Self {
   Default::default()
}

which calls
impl Default for Context {
which initializes the given builtins (which are mostly ES builtins).


Through this path I would pass a parameter which would contain the list of external structs with init callback method.
Each instance from the list will have its init called, which will receive the global_object that it will be able to inject into.

Or use some other injection strategy through this path.

@Razican Razican added this to the v0.12.0 milestone Jan 11, 2021
@Razican Razican modified the milestones: v0.12.0, v0.13.0 May 22, 2021
@raskad raskad modified the milestones: v0.13.0, v0.14.0 Sep 25, 2021
@Razican Razican modified the milestones: v0.14.0, v0.15.0 Feb 23, 2022
@Razican Razican modified the milestones: v0.15.0, v0.16.0 Jun 1, 2022
@Razican Razican moved this to To do in Boa pre-v1 Sep 7, 2022
@Razican Razican modified the milestones: v0.16.0, v0.17.0 Sep 19, 2022
@Razican Razican self-assigned this Mar 25, 2023
@bors bors bot closed this as completed in 63d9d67 Apr 24, 2023
@github-project-automation github-project-automation bot moved this from To do to Done in Boa pre-v1 Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants