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

Editorial: Explicitly define ambiguous namespace properties as undefined #2401

Closed

Conversation

guybedford
Copy link

This ensures that the spec is explicit about ambiguous bindings on module namespaces that have not otherwise been imported in a way that would throw an error will always be undefined on the namespace, effectively skipping the unnecessary call to _targetEnv_.getBindingValue.

See issue #2399 for the exact details and scenario to which this applies.

Comment on lines +12042 to +12043
1. If _binding_.[[BindingName]] is *"\*ambiguous\*"*, then
1. Return *undefined*.
Copy link
Member

Choose a reason for hiding this comment

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

would it be worth exploring the web compatibility of making this throw?

Copy link
Author

Choose a reason for hiding this comment

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

It's difficult to say, but it likely is a web compat issue at this point I'd guess. It could still be worth discussing further though. Another concern about throwing would be even just logging a namespace to the console would be enough to cause the error.

@anba
Copy link
Contributor

anba commented May 3, 2021

There seems to be some kind of misunderstanding around ambiguous exports:

  1. The string "*ambiguous*" is used nowhere within the spec.
  2. The spec does use the string "ambiguous" (so without the asterisk).
  3. "ambiguous" is a possible return value in ResolveExport, but in that position it's never used as a [[BindingName]] value in a ResolvedBinding record.
    • ResolveExport has three different return values:
      1. A ResolvedBinding record.
      2. null
      3. and "ambiguous".
  4. Module namespace objects return undefined for ambiguous exports due to steps 4-5:
  1. Let exports be O.[[Exports]].
  2. If P is not an element of exports, return undefined.
  1. The [[Exports]] slot of module namespace exports is set in ModuleNamespaceCreate. This list doesn't contain any ambiguous exports by definition, cf. GetModuleNamespace where the exports list is created.

@guybedford
Copy link
Author

@anba thanks for the very clear explanation here, you are entirely correct! Yes, I missed that these were being filtered off the namespace by the string value "ambiguous" not being a ResolveBinding record.

@guybedford guybedford closed this May 3, 2021
@guybedford guybedford deleted the explicit-undefined-ambiguous branch May 3, 2021 22:19
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.

None yet

3 participants