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

TS3.0-RC - VRLayer left/rightBounds type. #26038

Closed
cmerighi opened this issue Jul 28, 2018 · 11 comments
Closed

TS3.0-RC - VRLayer left/rightBounds type. #26038

cmerighi opened this issue Jul 28, 2018 · 11 comments
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue

Comments

@cmerighi
Copy link

TypeScript Version: 3.0.0-rc
VRLayer type described in 3.0rc's lib.dom.d.ts conflicts with the one in @types/webvr-api. The latter is a devDependency for the widely used @types/three.

VRLayer in lib.dom.d.ts:

interface VRLayer {
    leftBounds?: number[] | Float32Array | null;
    rightBounds?: number[] | Float32Array | null;
    source?: HTMLCanvasElement | null;
}

VRLayer in @types/webvr_api/index.d.ts:

interface VRLayer {
    leftBounds?: number[] | null;
    rightBounds?: number[] | null;
    source?: HTMLCanvasElement | null;
}

Looking at the specs, including Float32Array among the piped types seems unnecessary/wrong to me.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 30, 2018

This all came about due to an issue discussed in microsoft/TypeScript-DOM-lib-generator#419 where some browsers adopted earlier versions of the spec which required Float32Array.

@mhegazy mhegazy added Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue Fixed in TSJS repo Fix merged in https://github.com/Microsoft/TSJS-lib-generator, but not ported yet labels Jul 30, 2018
@mhegazy mhegazy self-assigned this Jul 30, 2018
@mhegazy mhegazy added this to the TypeScript 3.1 milestone Jul 30, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jul 30, 2018

Should be fixed by #25944

@abirmingham
Copy link

abirmingham commented Jul 30, 2018

Is there a workaround for this issue in the meantime? Thanks!

EDIT: Worked around this issue by adding a new type roots folder, with webvr-api/index.d.ts and the Float32Array definition. Will subsequently drop this folder and the erroneous definition.

@efokschaner
Copy link

@mhegazy perhaps I'm missing it but I don't see any changes to VRLayer in the pr you mentioned #25944 , are you sure it fixes it?

@uniqueiniquity
Copy link
Contributor

@RyanCavanaugh

@kitsonk
Copy link
Contributor

kitsonk commented Aug 20, 2018

It looks like microsoft/TypeScript-DOM-lib-generator#546 hasn't been merged.

@efokschaner
Copy link

@kitsonk that pr still seems to include Float32Array in the union https://github.com/Microsoft/TSJS-lib-generator/pull/546/files#diff-a97a9d0dbf71ec2f96be57f7fb9f70c9R1523

So we would still be seeing the same issues we're having right now with the webvr typings, no?

I'd still appreciate some input on how best to reconcile this situation perhaps from @mhegazy or @RyanCavanaugh .

There are three situations that need to be handled by the DT webvr-api typings:

  • Use with TS before it had any webvr typings built in.
  • Use with later 2.x TS versions which had webvr typings.
  • Use with the 3.x TS versions with this changed definition.

Is there any way to satisfy all three? Or should we do a major version bump of the webvr typings and support TS 3 onwards only in the next major version? I've not done a major version bump inside DT before, anything to be aware of if that's our preferred choice here? Is there still a possibility TS 3.x will revert to how it was in 2? @mhegazy and @kitsonk both seem to suggest this may be happening though I'm yet to see a code change that does it.

@efokschaner
Copy link

@mhegazy, perhaps we should re-open this issue? We still have the exact issue described. Do you remember what was the motivation for closing the issue?

@markgoho-EDT
Copy link

Why is this issue closed when it hasn't been resolved?

SirVer added a commit to SirVer/point_cloud_viewer that referenced this issue Sep 20, 2018
wally-the-cartographer pushed a commit to cartographer-project/point_cloud_viewer that referenced this issue Sep 20, 2018
This bug is not in Typescript < 3.0, so we force a version between 2.0 and 3.0 to not get hit by this bug in ci. Once it is fixed we can update to > 3.* eventually again.

microsoft/TypeScript#26038
@abirmingham
Copy link

Bump. Still running into this issue with 3.1.1.

@cmerighi
Copy link
Author

cmerighi commented Oct 2, 2018

Bump. Still running into this issue with 3.1.1.

Updating the dependencies to the following versions, everything runs smoothly:

"devDependencies":{
    "typescript": "3.1.1",
    "@types/three": "^0.92.23",
    /*"@types/webvr-api": "^0.0.34" removed */
    /* [...] others */
}

Given that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

7 participants