-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
feat(lib): define TypedArray interface #59205
Conversation
Each concrete typed array type and constructor share common interfaces. Library types can be defined extending base interfaces: 1. higher code reuse 2. less duplication 3. easier maintenance (less error prone) Closes microsoft#15402 Fixes microsoft#45198
The TypeScript team hasn't accepted the linked issue #15402. If you can get it accepted, this PR will have a better chance of being reviewed. |
@typescript-bot test it |
Hey @DanielRosenwasser, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: pouchdb-core
Package: node/v16
Package: node/v18
Package: node
|
@DanielRosenwasser Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
@DanielRosenwasser Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
This PR does partly the same I think, #58188 - but is less breaking? |
This PR does address #45198, I'm not sure if it addresses #45199 though. Oh, on second look though, this might address it too. I can try grabbing the test changes from https://github.com/microsoft/TypeScript/pull/58188/files#diff-259a5a5be5d9d2b11bd636646375791a0d37a60f80c1871b6b97c9f1cde672ee to confirm. |
The main objective of this PR is more to create a common interface for all |
@DanielRosenwasser Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
@DanielRosenwasser Here are some more interesting changes from running the top 400 repos suite Details
|
Changes TypedArray methods to receive/return the concrete subclass or `this` rather than only `this` which breaks `Buffer` in `@types/node` (e.g. `slice()` in `Buffer` returns `Buffer` and not `this`).
@typescript-bot test it |
@DanielRosenwasser, I've pushed a potential fix for |
@typescript-bot test it |
Starting jobs; this comment will be updated as builds start and complete.
|
I'm also interested to see what performance looks like with a second sample. |
There are merge conflicts, so the bot can't run anything. |
That and there's an Azure DevOps outage.... |
merge conflicts resolved; this branch is now up to date with
can we try again @sandersn? |
@typescript-bot test it |
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
What do you think of these changes @sandersn? This branch is currently up to date with |
Sorry, I misspoke. I mistakenly thought you were referencing #59407, which is similar except that #59407 allows the individual typed arrays to be generic over The only way this PR intersects with #58573 would be in what paths are used. |
Possibly, but I'm not certain #59407 is the best solution either. While I'd like to introduce |
Each concrete typed array type and constructor share common interfaces.
Library types can be defined extending base interfaces:
Closes #15402
Fixes #45198