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

Ensure object creation specifies the realm #810

Merged
merged 3 commits into from
Jan 29, 2025

Conversation

inexorabletash
Copy link
Member

@inexorabletash inexorabletash commented Jan 28, 2025

"Realm" is an ECMAScript concept best explained in https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts Newly created JS objects must be associated with a Realm; while older specs didn't do this explicitly, best practice is to be explicit about this, especially for steps running "in parallel", or in algorithms separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps" it the body of spec algorithms, not JS objects, so they don't have a realm. Conversion to a JS object when returning a dictionary to script is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection values, are not given a realm. This is a known issue across all web specs and is tracked in whatwg/webidl#135.

Resolves #793.


Preview | Diff

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.
@inexorabletash
Copy link
Member Author

@fdwr @huningxin - please take a look? Low priority.

Copy link
Collaborator

@fdwr fdwr left a comment

Choose a reason for hiding this comment

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

One q, else LGTM.

index.bs Show resolved Hide resolved
index.bs Outdated
@@ -1279,7 +1282,8 @@ The <dfn>context lost</dfn> steps for {{MLContext}} |context|, are:
<summary>
To <dfn for=MLContext>lose</dfn> {{MLContext}} |context| with {{DOMString}} |message|:
</summary>
1. Let |info| be a new {{MLContextLostInfo}}.
1. Let |realm| be |context|'s [=relevant realm=].
1. Let |info| be a new {{MLContextLostInfo}} in |realm|.
Copy link
Member Author

Choose a reason for hiding this comment

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

On re-reading, this is a dictionary and so also doesn't need the Realm. I think it's harmless, but to be consistent I removed it in aa9b161

Copy link
Collaborator

@fdwr fdwr left a comment

Choose a reason for hiding this comment

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

👍

tools/lint.mjs Outdated Show resolved Hide resolved
Copy link
Contributor

@huningxin huningxin left a comment

Choose a reason for hiding this comment

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

LGTM!

@fdwr
Copy link
Collaborator

fdwr commented Jan 29, 2025

Thanks Josh.

@fdwr fdwr merged commit 2139976 into webmachinelearning:main Jan 29, 2025
2 checks passed
github-actions bot added a commit that referenced this pull request Jan 29, 2025
SHA: 2139976
Reason: push, by fdwr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
zolkis pushed a commit to zolkis/webnn that referenced this pull request Feb 6, 2025
* Ensure object creation specifies the realm

"Realm" is an ECMAScript concept best explained in
https://html.spec.whatwg.org/multipage/webappapis.html#realms-and-their-counterparts
Newly created JS objects must be associated with a Realm; while older
specs didn't do this explicitly, best practice is to be explicit about
this, especially for steps running "in parallel", or in algorithms
separate from method steps. Do so!

This also adds lint tests to try and catch future violations. Note
that dictionaries (e.g. MLOperatorDescriptor) are Infra "ordered maps"
it the body of spec algorithms, not JS objects, so they don't have a
realm. Conversion to a JS object when returning a dictionary to script
is handled by WebIDL bindings logic.

Also note that DOMExceptions, either thrown or as promise rejection
values, are not given a realm. This is a known issue across all web
specs and is tracked in whatwg/webidl#135.

Resolves webmachinelearning#793.

* Don't double-init realm; and don't need realm for dicts

* Variable name improvement from @fdwr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create new objects with a Realm
3 participants