jj101k / eximport

0 stars 0 forks source link

Recursive mutual import can produce invalid state #9

Closed jj101k closed 4 years ago

jj101k commented 4 years ago

TypeError: Class extends value undefined is not a constructor or null

(Edited to correct bad report)

a.js:

import { B } from "./b"
export class A {}

b.js:

import { C } from "./c"
export class B {}

c.js:

import { A } from "./a"
export class C extends A {}

The class extension fails on C

jj101k commented 4 years ago

New plan: use with() and getters.

In this scenario, the final value of B isn't needed in c.js so it should be able to return immediately, which will let b.js return immediately, which will mean that there is a value to extend. Kind of a dependency maze.

While with() itself is pretty bad, it should do the job.

jj101k commented 4 years ago

Oh, c.js is wrong, should be:

import A from "./a"
export class C {
  get Bar() {
    return new A()
  }
}

...but it's failing anyway

jj101k commented 4 years ago

Oh, big misdiagnose here: all the imports are missing curlies

jj101k commented 4 years ago

I wish I'd committed the offending state now

jj101k commented 4 years ago

Got a reduced approximation, where:

a <- b <- c <- a(*)

Problem appears in c

jj101k commented 4 years ago

a.js:

import { B } from "./b"
export class A {}

b.js:

import { C } from "./c"
export class B {}

c.js:

import { A } from "./a"
export class C extends A {}
jj101k commented 4 years ago

Corrected original comment

jj101k commented 4 years ago

So we hit, via Node's require rules:

  1. A1 (require)
  2. B1 (require)
  3. C1 (require; node require loop mitigation kicks in and the process continues)
  4. C2 (crash)
jj101k commented 4 years ago

As long as the requires remain as they are, the first three steps are unavoidably the same. To work, A2 must appear before C2 (and after A1).

jj101k commented 4 years ago

Last week I thought through this and jotted down some notes:

Use JS generators

Try for “import * as” first, because less special foo needed. Try with() later

In principle, just a wrapping key-value generator would do with it being aborted early as needed.

The bridge would fundamentally need bridge.handle(function() {...}) on the exporting module.

All getters should be set initially (before any require), which will mean that the loop case should have the means to resume, and in particular that the crash module will return before crashing.

So:

A1, b1, c1, a2

Since non-export cases might not otherwise be resumed they should still try to commit at the end, but that should just run the rest of the generator

jj101k commented 4 years ago

Having quick-built an attempted implementation it’s getting in a generator loop - which is to say that the generator is being called inside itself.

There might be some silliness like the generator being restarted when reloading but I think because currently it stops at the first opportunity not the first required point it may be trying to step further when it doesn’t strictly need to

jj101k commented 4 years ago

Or perhaps:

A: import b; stop B: import c; stop C: import a; stop A: export a...

jj101k commented 4 years ago

So it's in the middle of a generator step - has not reached the await point on the "import b" line - when "a" is once more imported. Or so it seems.

jj101k commented 4 years ago

Oh, or it uses it at that stage. The generator is being pushed in the getters

jj101k commented 4 years ago

Because it isn't actually _await_ing^W yield ing at import

jj101k commented 4 years ago

So the problem is that these are in the same hunk:

import { B } from "./b"
export class A {}

Or rather that these are:

import { A } from "./a"
export class C extends A {}
jj101k commented 4 years ago

Now need the require() to finish(?) before it reaches the second line of the file, but also want the second line of the file to be evaluated before any line following require().

jj101k commented 4 years ago

Popping out the commit might do the job though, as long as it's tucked after the yield

jj101k commented 4 years ago

Need to consider:

export var a = 1
a = 2
jj101k commented 4 years ago

with() seems viable, and late assignment of mutable values is actually already covered (though deep assignment on immutable values probably is not)

jj101k commented 4 years ago

Regression on:

import * as aAll from "./export/a"

var a=1, b=2, c=3, d=4;
export { a, b };
export { c as c1, d };

export default 6 * 7;
jj101k commented 4 years ago
> const eximportBridge=require("eximport-bridge").bridge;eximportBridge.prepare(["default","a","b","c1","d"]);module.exports=eximportBridge.ns;eximportBridge.handle(function* () {var a=1, b=2, c=3, d=4;
> undefined;
> undefined;
> 
> yield eximportBridge.ns.default=6 * 7;
> ;eximportBridge.commit({})})

Uhhh

jj101k commented 4 years ago

For var that actually works, but bug: that won't work for things like:

export const a = []
a.push(2)