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

ReadableStreamReadResult interface is private #42970

Closed
johnBartos opened this issue Feb 25, 2021 · 4 comments
Closed

ReadableStreamReadResult interface is private #42970

johnBartos opened this issue Feb 25, 2021 · 4 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@johnBartos
Copy link

johnBartos commented Feb 25, 2021

Bug Report

πŸ”Ž Search Terms

ReadableStream, ReadableStreamReadResult

πŸ•— Version & Regression Information

  • This changed between versions <= 4.1.5 and 4.2.x

⏯ Playground Link

Playground link with the bug (4.2.x)

Playground link without the bug (4.1.5)

πŸ’» Code

        const handleChunk = ({ done, value }: ReadableStreamReadResult<Uint8Array>) => {
            console.log(done, value);
        }

πŸ™ Actual behavior

  • Cannot find name 'ReadableStreamReadResult'.
  • Return type of public method from exported class has or is using private name 'ReadableStreamReadResult'.

πŸ™‚ Expected behavior

No type error

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Mar 4, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.3.1 milestone Mar 4, 2021
@sandersn
Copy link
Member

sandersn commented Apr 6, 2021

Looking at the PR microsoft/TypeScript-DOM-lib-generator#890, it looks like the name changed to ReadableStreamDefaultReadResult. Does that work for you @johnBartos?

@tchakabam
Copy link

tchakabam commented Apr 22, 2021

@sandersn Not sure about @johnBartos ' case. But your above hint helped around here: Renaming the type occurence to ReadableStreamDefaultReadResult.

Am I correct in understanding this would actually be a breaking change from a minor version bump of tsc (a bug in that sense) ? This occured here as well from re-installing on a "typescript": "^4.1.5" package dev-deps (where probably 4.2.x got installed then).

EDIT: Just being curious checked the web spec status, which however is inline with this change in the type-name, see https://streams.spec.whatwg.org/#dictdef-readablestreamdefaultreadresult

However I don't understand why on a minor version bump the compiler doesn't provide a backward compatibility alias type here ... Even if you could deprecate the prior name somehow, adding a warning or so?

@MattiasBuelens
Copy link
Contributor

First off all: I'm sorry for the inconvenience. I was the one who contributed this change to TypeScript 4.2.

In earlier versions of the Streams standard, many types didn't have "official" definitions with standardized names. So when I wrote these type definitions, I took a best-effort guess to name them based on what we had earlier in @types/whatwg-streams. When the Streams standard was finally updated with official WebIDL definitions for all types, some of these names didn't match my guess, hence I had to rename them.

Am I correct in understanding this would actually be a breaking change from a minor version bump of tsc (a bug in that sense) ? This occured here as well from re-installing on a "typescript": "^4.1.5" package dev-deps (where probably 4.2.x got installed then).

As far as I know, TypeScript doesn't follow semantic versioning. If you want to stay on the same minor version, you should use a tilde constraint like ~4.1.0 (example).

It's definitely not unusual for TypeScript to make breaking changes in the DOM types between minor releases. Pretty much every release notes article has a "lib.d.ts updates" section, giving a (non-exhaustive) list of changed types. The changes to the Streams types fit in that scheme.

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Jun 18, 2021
@tchakabam
Copy link

@MattiasBuelens Thx for the context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

No branches or pull requests

6 participants