`Node` objects can't be lifted but are considered phase independent

yoav-steinberg commented 6 months ago

I tried this:

let x = nodeof(this);
test "test" {
  log(x.addr);
}

This happened:

Error: Unable to lift object of type Node
  --> temp/main.w:1:1
1 | let x = nodeof(this);
  | ^
at _toInflightType → _toInflight /home/yoav/wing/temp/main.w:1:1

Tests 1 failed (1)
Snapshots 1 skipped
Test Files 1 failed (1)

I expected this:

To print the node address

Is there a workaround?

Don't use the node object inflight, lift the specific fields you need (which makes more sense anyway)

let x = nodeof(this).addr;
test "test" {
  log(x);
}

Anything else?

Note that this use to work back when we more eagerly lifted expressions, so x.addr was lifted in its entirety while today x is lifted (being phase independent) and addr is accessed inflight. Tthis is good for many reasons but introduces this bug where phase independent classes that don't implement ILiftable (like stuff imported from JSII) cause an issue. Ideas for solving this:

Mark all not wingsdk imported JSII classes as preflight, regardless of whether they are constructs or not. This isn't ideal since some classes can be instantiated and used inflight. And doing so will limit the usefulness of using JSII as a generic library system for wing.
Serialize these classes, auto implement ILiftable for them, is this possible? How?
Make the type checker aware that we're trying to lift a non-liftable and just give a good type error for this.
Partial fix: today anything that doesn't inherit from Construct is considered phase independent, but we might want to consider anything that doesn't have any preflight types in its surface area as phase independent. the std.Node objects has multiple methods, and fields (root, of()) that reference the preflight Construct type. So perhaps the Node type should be marked as preflight after all. This will fix this specific case, but not the generic issue of not being able to lift phase independent objects/types brought from JSII.

Wing Version

No response

Node.js Version

No response

Platform(s)

No response

Community Notes

Please vote by adding a 👍 reaction to the issue to help us prioritize.
If you are interested to work on this issue, please leave a comment.

yoav-steinberg commented 6 months ago

Another example of the same issue using our JSII fixture class which doesn't have any "constructs" in it:

bring "jsii-fixture" as jf;
let x = new jf.JsiiClass(1);
test "test" {
  log("{x.field()}");
}

This causes the same "can't lift object of type JsiiClass" issue.

eladb commented 6 months ago

Will this work?

let o = new JsiiClass();

inflight () => {
  log(o.myString);
};

yoav-steinberg commented 6 months ago

Will this work?

No, it won't, it's the same as my example above. JsiiClass is phase ind but doesn't implement ILiftable so o can't be lifted. In theory we can eagerly lift o.myString but in some cases we don't want to do that because we want to access myString in inflight (because of any side effects of this access, they use would probably expect it to be accessed inflight).

eladb commented 6 months ago

because of any side effects of this access

What kind of side effects can occur?

github-actions[bot] commented 3 months ago

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

winglang / wing