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

Ambiguous namespace export access returns undefined #2399

Closed
guybedford opened this issue May 2, 2021 · 2 comments
Closed

Ambiguous namespace export access returns undefined #2399

guybedford opened this issue May 2, 2021 · 2 comments

Comments

@guybedford
Copy link

Currently ambiguous export bindings that have no other importer but that are available on a module namespace will always return undefined.

Example scenario from rollup/rollup#4049 (comment):

// index.js
import * as ns from './reexport.js';
console.log(ns.foo, ns.bar, ns.baz);

// reexport.js
export * from './first.js';
export * from './second.js';

// first.js
export const foo = 1;
export const bar = 1;

// second.js
export const foo = 2;
export const baz = 2;

Output:

undefined 1 2

The undefined here seems to be weakly specified in the modules specification per https://tc39.es/ecma262/#sec-module-namespace-exotic-objects-get-p-receiver, where [[BindingName]] "*ambiguous*" is not explicitly handled in the steps but implicitly returns undefined when passed into targetEnv.[[GetBindingValue]].

The scenario above directly corresponds to the last point mentioned in the spec note in https://tc39.es/ecma262/#sec-getmodulenamespace:

The only way GetModuleNamespace can throw is via one of the triggered HostResolveImportedModule calls. Unresolvable names are simply excluded from the namespace at this point. They will lead to a real linking error later unless they are all ambiguous star exports that are not explicitly requested anywhere.

Because all of the ambiguous exports have no other direct importer, and only a namespace representation, the ambiguous error is not thrown during initial linking.

Ideally this namespace Get undefined return should be explicitly specified here. Alternatively an ambiguous error could be thrown on access, although it's unclear whether that would be web compatible at this point.

@guybedford
Copy link
Author

I've posted an editorial PR in #2401 to more clearly define this behaviour.

@guybedford
Copy link
Author

Closing per #2401 (comment). Apologies for noise. The good news at least is that RollupJS is now fully compatible with these behaviours down to some warnings provided during externalization where ambiguity can't be known ahead of time.

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

No branches or pull requests

1 participant