Fix typing for 'Lifted<T>' to work better across versions of TS #3658
Conversation
| // Output<T> = OutputInstance<T> & Lifted<T> | ||
| // so returning an empty object just means that we're adding nothing to Output<T>. | ||
| // This is needed for cases like `Output<any>`. | ||
| {}; |
There was a problem hiding this comment.
this happened due to the removal of T extends Resource ? { } : ... that came in: ad4da29#diff-b7a9c3c1793336a377df01cb5e0e9f40
Previously, the any cases in Output<any> would be caught by this, effectively adding no members to Output<any>. i.e. Output<any> = OutputInstance<any> & {} TS seems to have refined things slightly so that this now becomes Output<any> = OutputInstance<any> & Lifted<{}> which it feels is no longer compatible because objects are indexable.
My theory is that this fell out of TS 3.7's recursive type aliases work (something which we could benefit from). Basically, instead of aggressively flattening the type, they lazily do it, but it means that they can see different types here which now triggers some subtle assignability issue.
Regardless, the above should be strictly better and works downlevel and on 3.7. It does really indicate what we want, namely that only 'objects/arrays' should have their properties lifted up, and anything else should not participate in lifting.
| @@ -3,7 +3,7 @@ | |||
| "version": "${VERSION}", | |||
| "license": "Apache-2.0", | |||
| "dependencies": { | |||
| "@pulumi/pulumi": "latest" | |||
| "@pulumi/pulumi": "=1.6.0" | |||
There was a problem hiding this comment.
note: i need to lock against pulumi 1.6 because 1.7.1 is not compatible with 1.7.0 (because of this very issue we are fixing). Effectively 1.7.0 is hte screwup one that we need to overlook. Once 1.7.1 is in, we can unlock this and validate that future versions are compatible with 1.7.1
| # will be renabled once that goes in. | ||
| # pushd tests/sxs && yarn && tsc && popd | ||
| pushd tests/sxs_ts_3.6 && yarn && tsc && popd | ||
| pushd tests/sxs_ts_latest && yarn && tsc && popd |
There was a problem hiding this comment.
updated to actually test different versions of ts. I don't like that this is done with copy/pasting the test. ideally we could truly have a matrix here where we test a whole bunch of combinations. But i think this approach is acceptable for now given the time and amount of work we have.
CyrusNajmabadi
changed the title
This should likely go out in a 1.7.1 release.
This addresses the issue that with some versions of TS, using
@pulumi/pulumisxs with previous versions could cause assignability errors betweenOutput<any>andOutput<any>.