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

Add support for ObjectTemplate::SetAccessorProperty #90

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

stroiman
Copy link

Finally wrote some tests for this baby :)

There's thing in the interface I'm unsure of. I expose the basic SetAccessorProperty which requires FunctionTemplate instances as input.

But mostly, you don't need this instance. That would only be if you actually use the same function object in multiple places, but that's not the normal usage pattern.

So I also created a helper on top, SetAccessorPropertyCallback, which creates the templates. I'm unsure if this should be in the interface. I find it helpful, but I have no objection to remove it.

Regarding the flags, I haven't researched what they do. I discovered, you shouldn't use ReadOnly. I think the only flag that is applicable for this case is DontEnum, which I believe would prevent the property from being iterated in for . in loops. But it's still listed by Object.getOwnPropertyNames. But I didn't validate this, so I didn't add to the documentation.

@tommie
Copy link
Owner

tommie commented Feb 8, 2025

// READ_ONLY is an invalid attribute for JS setters/getters.

in src/objects/objects.cc, so I think you're right. Seems this may be about const and module exports.

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.

2 participants