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

fix(dts): array buffer types (ES2024,ES2017) #57858

Closed
wants to merge 3 commits into from
Closed

fix(dts): array buffer types (ES2024,ES2017) #57858

wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 19, 2024

This should be an updated, and hopefully correct fix from the stale PR here #54637.

In summary this PR is an extension of the previous PR with several modifications (to comments, interfaces, references etc.) The previous PR included ArrayBuffer-related interfaces that are part of ES2024.

It passes most of the tests with the exceptions of baselines from what I understand, so those are accepted and send with a commit.

Would be useful to get some feedback if this looks good in principle @petamoriken @sandersn, I've included most of the reviews you made in the stale PR that I linked earlier.

Fixes #54636

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Mar 19, 2024
@ghost
Copy link
Author

ghost commented Mar 20, 2024

I'm unsure what the licence agreement implies about original code, part of this PR is based on a stale PR linked at the top, since it's impossible to rebase the original one (it's 800 commits behind.)

@jakebailey
Copy link
Member

Not a lawyer, but they gave the PR away in #54637 (comment), so I would just add Co-authored-by: ... to your commit to credit the original code.

@ghost
Copy link
Author

ghost commented Mar 21, 2024

Not a lawyer, but they gave the PR away in #54637 (comment), so I would just add Co-authored-by: ... to your commit to credit the original code.

Thanks, sounds good. I can only do that revising and forcing a push I think. Is this fine for you?

I will accept the CLA after doing that.

@jakebailey
Copy link
Member

Yes, feel free to force push

@ghost
Copy link
Author

ghost commented Mar 21, 2024

Yes, feel free to force push

done, it did work because the profile picture is correct in the co-authored commit.

@ghost
Copy link
Author

ghost commented Mar 21, 2024

@microsoft-github-policy-service agree

@ghost
Copy link
Author

ghost commented Mar 21, 2024

Still needs some tests #54637 (review)

and also for the new def of slice() and the empty ArrayBuffer and such constructors

@BlackAsLight
Copy link

Any update on this?

@ghost
Copy link
Author

ghost commented Apr 5, 2024

Any update on this?

I can't spend the time now (tendonitis.) I think it needs some tests only. Feel free to take over the PR.

Comment on lines +1 to +17
interface ArrayBuffer {
/**
* If this ArrayBuffer is resizable, returns the maximum byte length given during construction; returns the byte length if not.
*/
get maxByteLength(): number;
/**
* Returns true if this ArrayBuffer can be resized.
*/
get resizable(): boolean;

/**
* Resizes the ArrayBuffer to the specified size (in bytes).
* @param newByteLength The new length, in bytes, to resize the ArrayBuffer to.
*/
resize(newByteLength: number): undefined;
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are other methods and properties that need to be added. Like this: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer/detached

@ghost ghost closed this by deleting the head repository May 16, 2024
@petamoriken
Copy link
Contributor

Oh, It appears that the account has been lost. I'll take it instead.

@guest271314
Copy link

@petamoriken Status?

@petamoriken
Copy link
Contributor

@guest271314 Please see #58573

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix(dts): Update type of ArrayBuffer
7 participants