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

Clarify the purpose of JSClass #66

Open
wants to merge 1 commit into
base: esr102
Choose a base branch
from

Conversation

gschwind
Copy link

The current document does not explain the usage of JSClass clearly. This patch try to give more detail about the purpose of JSClass, how to use it, and it's internal constraint.

Note that I did this documentation based on my own investigation within the current spidermonkey code, thus I maybe wrong.

The current document does not explain the usage of JSClass clearly. This
patch try to give more detail about the purpose of JSClass, how to use
it, and it's internal constraint.
Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Very neat, I was not aware that you can actually create a JS class in C++ where the instances and the prototype don't share the same JSClass and JSClassOps! I always thought it was a peculiarity of JSClass that the instances and prototype had to have the same one.

If this is correct, I think it means we can simplify a bunch of other code. For example, we should make the same change in examples/resolve.cpp and that means we can remove the isPrototype() checks from the JSClassOps callbacks.

@jandem
Copy link
Contributor

jandem commented Jan 2, 2023

I always thought it was a peculiarity of JSClass that the instances and prototype had to have the same one.

It used to be that builtin prototype objects in JS had the same type as the instance (e.g. Array.prototype is an array object in JS). The spec has been moving away from that and now generally prefers plain objects for prototypes.

JS_InitClass still uses the JSClass for both the proto and constructor name. It would probably make sense to change this API to default to using a plain object for the prototype.

@jandem
Copy link
Contributor

jandem commented Jan 2, 2023

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1808171 to improve JS_InitClass.

@jandem
Copy link
Contributor

jandem commented Jan 4, 2023

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1808171 to improve JS_InitClass.

These changes have now landed, so with the next ESR release we can improve this more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants