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

feat(ecmascript): TypedArrayCreateFromConstructor and %TypedArray%.of #601

Merged

Conversation

Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

Thank you, and great work!

Found some issues around GC, and we can split the method up a bit to make it cleaner in a sense. (Oh, we could also theoretically do a fast-path for when all the parameters given are numbers or bigints, depending on the TypedArray kind; in those cases we could probably mostly skip the whole Set internal method. But that's involved and somewhat error prone, so let's leave that for another day! :D )

@yossydev yossydev force-pushed the feat/typed_array_create_from_constructor branch from 21a4ab2 to 1f6430b Compare March 19, 2025 17:21
@yossydev yossydev force-pushed the feat/typed_array_create_from_constructor branch from 1f6430b to 52aff10 Compare March 19, 2025 17:24
@yossydev yossydev requested a review from aapoalas March 19, 2025 17:34
Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

Damn, sorry I lead you astray a bit.

@yossydev yossydev requested a review from aapoalas March 20, 2025 05:09
Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

Still a bit smoothing of edges to do :)

@yossydev yossydev requested a review from aapoalas March 25, 2025 11:42
@load1n9 load1n9 self-requested a review March 25, 2025 17:14
Copy link
Member

@load1n9 load1n9 left a comment

Choose a reason for hiding this comment

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

LGTM :) but remove the index.js file before merging

@aapoalas aapoalas requested a review from load1n9 March 25, 2025 19:52
@aapoalas aapoalas requested review from load1n9 and removed request for load1n9 March 25, 2025 19:53
@aapoalas aapoalas dismissed load1n9’s stale review March 25, 2025 19:54

Removed index.js file.

@aapoalas aapoalas merged commit 73511cc into trynova:main Mar 25, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants