-
Notifications
You must be signed in to change notification settings - Fork 12.2k
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
Symbol.species should on constructor, not instance #50167
Conversation
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
src/lib/es2017.sharedmemory.d.ts
Outdated
readonly [Symbol.toStringTag]: "SharedArrayBuffer"; | ||
} | ||
|
||
interface SharedArrayBufferConstructor { | ||
readonly prototype: SharedArrayBuffer; | ||
new (byteLength: number): SharedArrayBuffer; | ||
readonly [Symbol.species]: SharedArrayBuffer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this also be SharedArrayBufferConstructor
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielRosenwasser Oh... Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening this up and Node.js and trying it at the console, I think this looks right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like you need to accept the baselines so tests pass first.
@DanielRosenwasser Thank you! I merged latest main from this repo, there is still error. Maybe I mistake your words "accept the baselines"? (I saw the error output has word "baseline" in the path, maybe "baseline" means other thing in this project?) |
After study, I think I can't handle the error information above, sorry! |
If you're not sure what to do, all that needs to be done is to run |
Do you want to keep working on this? Otherwise I'd like to close it to reduce the number of open PRs. |
Sorry, I can't... |
Fixes #50168