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

Adjust GetActiveScriptOrModule to shadow realms #415

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions spec.html
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,9 @@ <h1>ShadowRealm ( )</h1>
1. Return _O_.
</emu-alg>
</emu-clause>
<emu-note>
<p>The realm created in this algorithm is referred to as a <dfn>shadow realm</dfn>.</p>
</emu-note>
</emu-clause>

<emu-clause id="sec-properties-of-the-shadowRealm-constructor">
Expand Down Expand Up @@ -633,5 +636,36 @@ <h1>Requirements on host-defined global objects</h1>
</emu-clause>
</emu-clause>
</emu-clause>

<emu-clause id="sec-amendments">
<h1>Amendments to the ECMAScript® 2025 Language Specification</h1>

<emu-note type="editor">
<p>
This section lists amendments which must be made to <a href="https://tc39.es/ecma262/">ECMA-262, the ECMAScript® 2025 Language Specification</a>.
Text to be added is marked <ins>like this</ins>, and text to be deleted is marked <del>like this</del>.
</p>
</emu-note>

<emu-clause id="sec-getactivescriptormodule" type="abstract operation">
<h1>GetActiveScriptOrModule ( ): a Script Record, a Module Record, or *null*</h1>
<dl class="header">
<dt>description</dt>
<dd>It is used to determine the running script or module, based on the running execution context.</dd>
</dl>

<emu-alg>
1. If the execution context stack is empty, return *null*.
Copy link

@shannonbooth shannonbooth Nov 27, 2024

Choose a reason for hiding this comment

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

While this appears to work for HTML integration in the test cases that I have run it against so far (though, I have not tested extensively with importValue) unfortunately I have just found that it breaks some of the test-262 / test-js test cases (previously all test-262 cases were passing).

One of the failing test cases is this test case: https://github.com/LadybirdBrowser/ladybird/blob/5deb2b4cad3755c7e9251f6b23e9b1089b029aa7/Libraries/LibJS/Tests/builtins/ShadowRealm/ShadowRealm.prototype.importValue.js#L22

Where I am now getting:

 FAIL  builtins/ShadowRealm/ShadowRealm.prototype.importValue.js
    ❌ Suite:  normal behavior
         Test:   basic functionality (failed):
                 ExpectationError: Expected target to be null got TypeError: Cannot find/open module: './external-module.mjs'

I will try and investigate a bit more to find out what the problem is. Will also need to cross-check this against tc39/ecma262#3374

Copy link

@shannonbooth shannonbooth Nov 27, 2024

Choose a reason for hiding this comment

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

I will note that it is possible this is also a bug on my end in other areas of module loading outside of this change, but will just mention it incase

edit: I may have been incorrect earlier with saying that tc39/ecma262#3374 fixed the issue I was seeing, there may have been a specific case that it resolved, but on further testing I am running into issues with WPT tests with it. This change resolves the issues I am seeing with WPT, but seems to cause issues in the JS suite I am running against. tc39/ecma262#3374 does not break the JS test suite, but causes issues with WPT test cases. I need to do some further analysis to figure out what is going wrong...

1. Let _ec_ be the topmost execution context on the execution context stack whose ScriptOrModule component is not *null* <ins>or whose Realm is a shadow realm</ins>.
1. If no such execution context exists, return *null*. Otherwise, return _ec_'s ScriptOrModule.
</emu-alg>

<emu-note type="editor">
<p>
This adjustment will be obsoleted by <a href="https://github.com/tc39/ecma262/pull/3374">ECMA-262 PR 3374</a>.
</p>
</emu-note>
</emu-clause>
</emu-clause>
</body>
</html>