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

Error behaviour details #41

Closed
guybedford opened this issue Apr 4, 2024 · 1 comment · Fixed by #43
Closed

Error behaviour details #41

guybedford opened this issue Apr 4, 2024 · 1 comment · Fixed by #43

Comments

@guybedford
Copy link
Collaborator

guybedford commented Apr 4, 2024

Posting a summary from the modules meeting today:

  1. The current spec will not error for importing a deferred module which is already in an errored state. It might be worth making this case error early.
  2. If importing a sync module that imports an async module, it is possible for that sync module to be queued for execution by the time the promise completes either by an existing or future top-level evaluation. If we think of this in the same light as (1) above, the question then is whether the sync failure should also fail the deferred import as it can be known that the failure happened before the deferred import statically completes. If the sync module was already queued for execution we can directly attach to its completion. But if the sync module is only queued for execution later that case would be harder to track, since deferred attachments statically skip their direct sync graph components. An approach to this might be possible in the async fulfillment handlers to perform defer rejection when a defer parent "happens to be found".

In discussion we felt that (1) and (2) might be worth handling. Writing it up now I'm wondering if we should only consider (1) due to the complexity of (2), although it could be possible.

Further to the above, there is also the question of whether deferred namespaces should throw for every access or not when the module is in an errored state. There was tentative agreement on only the executor call being the one to throw.

@guybedford
Copy link
Collaborator Author

@nicolo-ribaudo happy to review any further PRs along these lines. Alternatively I'd be happy to propose some spec text if that would help, both for (1) and (2), just let me know what works best.

nicolo-ribaudo added a commit to nicolo-ribaudo/proposal-defer-import-eval that referenced this issue Apr 24, 2024
This commit changes the behavior of namespaces obtained through
`import defer` to re-throw evaluation errors even if the module
is fully evaluated:

```js
import defer * as ns from "module-that-throws";
try { ns.foo } catch { console.log("Error!") }
try { ns.foo } catch { console.log("Error!") }
```

The code above is now guaranteed to print `"Error!"` twice,
and it doesn't depend on whether other modules already evaluated
`"module-that-throws"`.

This is different from the existing behavior of namespace objects,
so we have to use a _different_ namespace object for deferred import:

```js
import defer * as ns1 from "module-that-throws";
import * as ns2 from "module-that-throws";

Promise.resolve().then(() => {
  try { ns1.foo } catch { console.log("Error1!") }
  try { ns2.foo } catch { console.log("Error2!") }
});
```

Assuming that this module's body is evaluated due to
a cycle, the code above will print `"Error1!"` and not
`"Error2!"`. This means that `ns1` !== `ns2`.
Namespace objects are still cached, so if you `import defer`
the same module twice you will get the same namespace object.

Thanks to this change, we can now also make "trying to evaluate
a deferred module in a cycle" an error, rather than silently
skipping its evaluation as done in tc39#39. This ensures that a binding
cannot be accessed _during evaluation_ to then disappear as soon
as the module is done evaluating with an error.

Fixes tc39#41 cc @guybedford
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant