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

Star exports and instantiation errors #722

Open
GeorgNeis opened this Issue Oct 27, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@GeorgNeis
Contributor

GeorgNeis commented Oct 27, 2016

When ResolveExport detects a cycle while trying to resolve a name through a star export, this by itself doesn't result in an error. For instance, consider these four modules (with A being the main module):

A: import {x} from "B";
B: export * from "C"; export * from "D";
C: export * from "B";
D: export let x;

The instantiation of this module graph succeeds. In particular, even though the C.ResolveExport call for x fails due to a cycle, the B.ResolveExport call for x succeeds because x can be resolved to the declaration in D. This makes sense to me, because noone is actually requesting x from C.

Now let's look at a different example:

A: import {x} from "B";
B: export * from "C"; export * from "D";
C: export * from "D"; export * from "E";
D: export let x;
E: export let x;

Note that C is ambiguous for x. But like before, noone is actually requesting x from C, so I would expect the B.ResolveExport call for x to succeed as well (resolving x to the declaration in D). According to the specification, however, it fails. What is the movitation for this difference?

@ajklein

This comment has been minimized.

Show comment
Hide comment
@ajklein
Contributor

ajklein commented Nov 1, 2016

@ajklein

This comment has been minimized.

Show comment
Hide comment
@ajklein
Contributor

ajklein commented Mar 10, 2017

@caridy

This comment has been minimized.

Show comment
Hide comment
@caridy

caridy Mar 10, 2017

Contributor

I will investigate.

Contributor

caridy commented Mar 10, 2017

I will investigate.

@caridy

This comment has been minimized.

Show comment
Hide comment
@caridy

caridy Mar 10, 2017

Contributor

Running this throw the algo it effectively return ambiguous for the second example:

First example:

A: import {x} from "B";
B: export * from "C"; export * from "D";
C: export * from "B";
D: export let x;

B: [{B, x}]
    C: [{B, x}, {C: x}]
        B: [{B, x}, {C: x}]
            -> returns null (step: 2.a.ii)
    D: [{B, x}, {C: x}, {D, x}]
        -> returns {[[Module]]: D, [[BindingName]]: x} (step: 9)
    -> returns {[[Module]]: D, [[BindingName]]: x} (step: 9)

Second Example:

A: import {x} from "B";
B: export * from "C"; export * from "D";
C: export * from "D"; export * from "E";
D: export let x;
E: export let x;

B: [{B, x}]
    C: [{B, x}, {C, x}]
        D: [{B, x}, {C, x}, {D, x}]
            -> returns {[[Module]]: D, [[BindingName]]: x} (step: 4.a.ii)
        E: [{B, x}, {C, x}, {D, x}, {E: x}]
            -> returns {[[Module]]: E, [[BindingName]]: x} (step: 4.a.ii)
        -> return "ambiguous" (step: 8.d.ii.2)
    -> return "ambiguous" (step: 8.c)

From the top of my head, there are two main motivations for this to return ambiguous:

  1. How will the developer know what binding they are importing? is it x from E or from D? at first glance, by looking at the code, it is impossible to tell.
  2. reordering export declarations in a module should have no effect on the exports, otherwise it is a refactor hazard.
Contributor

caridy commented Mar 10, 2017

Running this throw the algo it effectively return ambiguous for the second example:

First example:

A: import {x} from "B";
B: export * from "C"; export * from "D";
C: export * from "B";
D: export let x;

B: [{B, x}]
    C: [{B, x}, {C: x}]
        B: [{B, x}, {C: x}]
            -> returns null (step: 2.a.ii)
    D: [{B, x}, {C: x}, {D, x}]
        -> returns {[[Module]]: D, [[BindingName]]: x} (step: 9)
    -> returns {[[Module]]: D, [[BindingName]]: x} (step: 9)

Second Example:

A: import {x} from "B";
B: export * from "C"; export * from "D";
C: export * from "D"; export * from "E";
D: export let x;
E: export let x;

B: [{B, x}]
    C: [{B, x}, {C, x}]
        D: [{B, x}, {C, x}, {D, x}]
            -> returns {[[Module]]: D, [[BindingName]]: x} (step: 4.a.ii)
        E: [{B, x}, {C, x}, {D, x}, {E: x}]
            -> returns {[[Module]]: E, [[BindingName]]: x} (step: 4.a.ii)
        -> return "ambiguous" (step: 8.d.ii.2)
    -> return "ambiguous" (step: 8.c)

From the top of my head, there are two main motivations for this to return ambiguous:

  1. How will the developer know what binding they are importing? is it x from E or from D? at first glance, by looking at the code, it is impossible to tell.
  2. reordering export declarations in a module should have no effect on the exports, otherwise it is a refactor hazard.
@GeorgNeis

This comment has been minimized.

Show comment
Hide comment
@GeorgNeis

GeorgNeis Mar 27, 2017

Contributor

Thank you for having a look.

Regarding 1., the developer will know by looking at the specification. :)
I agree that it wouldn't be immediately clear at a first glance which x gets used, but that's already true for an example like the following:

A: import {x} from "B";
B: export * from "C"; export let x;
C: export let x;

Regarding 2., I'm not following. Reordering wouldn't make a difference here.

Contributor

GeorgNeis commented Mar 27, 2017

Thank you for having a look.

Regarding 1., the developer will know by looking at the specification. :)
I agree that it wouldn't be immediately clear at a first glance which x gets used, but that's already true for an example like the following:

A: import {x} from "B";
B: export * from "C"; export let x;
C: export let x;

Regarding 2., I'm not following. Reordering wouldn't make a difference here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment