Skip to content

Conversation

@sjorsdonkers
Copy link
Contributor

@sjorsdonkers sjorsdonkers commented Apr 22, 2025

Depends on: lightpanda-io/zig-v8-fork#57

Create an actually execution Context for Javascript to be executed in separately from the main context.

  • v8.contextCreated now has a parameter to indicated whether it is the default context, false prevents the inspector from adopting it.
  • isolated_world lives in BrowserContext as it is CDP specific. If we need to startScope / stopScope on page navigate we may reconsider where it lives.

sjorsdonkers

This comment was marked as resolved.

Base automatically changed from describeNode2 to main April 22, 2025 11:57
@sjorsdonkers sjorsdonkers force-pushed the new_isolated_world branch 2 times, most recently from 42e38fe to de6bb18 Compare April 22, 2025 13:25
// constructor/FunctionTemplate as normal, but also through the default
// FunctionTemplate of the isolate (in startExecutor)
fn attachClass(self: *Self, comptime Struct: type, template: v8.FunctionTemplate) void {
fn attachClass(self: *const Self, comptime Struct: type, template: v8.FunctionTemplate) void {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these functions should not be part of the struct

};

const Executor = struct {};
const Env = struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These generic CDPT interfaces seem to give us very little for the amount of complexity they introduce

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be possible to partially remove now and load the entire real env. Would probably still be a generic over the client though.

Comment on lines +486 to +487
name: []const u8,
grant_universal_access: bool,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not necessarily need to store these as they are not used at the moment.
Alternatively we could consider moving them into Executor as I expect that to merge towards being a Generic World type

@sjorsdonkers sjorsdonkers marked this pull request as ready for review April 22, 2025 15:09
if (comptime builtin.mode == .Debug) {
std.debug.assert(self.has_executor == false);
self.has_executor = true;
if (kind == .main) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In main, the debug has_executor check has been removed to accommodate this PR. With it removed, you should be able to remove kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When getting to this statement myself I initially thought about removing it as well, however I found I could also simplify the opening and closing of the context with it. The I also thought it would just be good metadata to have, so I'd prefer keeping it regardless.

@krichprollsch
Copy link
Member

@sjorsdonkers can you upgrade the zig-v8 dependency please?

@sjorsdonkers sjorsdonkers merged commit b2a975f into main Apr 23, 2025
12 checks passed
@sjorsdonkers sjorsdonkers deleted the new_isolated_world branch April 23, 2025 15:00
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants