-
-
Notifications
You must be signed in to change notification settings - Fork 135
fix: make has-trap more resilient #577
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
Conversation
|
Looking into this I think it needs a bit more logic to handle functions. What do you think of something like this? It would also be cool if you want to add some tests like you did in your other PRs :). We could use more tests to make sure this is correct. has(node: NodeInfo, prop: string | symbol) {
const value = getNodeValue(node);
// Short-circuit for the inputs that make Reflect.has error.
if (value === undefined || value === null) {
return false;
}
// Functions behave like objects here, so let them flow through.
if (typeof value === 'object' || typeof value === 'function') {
return Reflect.has(value, prop);
}
// For primitives (number, string, boolean, bigint, symbol) report “no key”.
// That keeps inspection code happy without pretending properties exist.
return false;
} |
3936bd2 to
092329f
Compare
|
I dive into the problem again and extract the essential parts as a unit test. I also add another needed change to get @vitest/pretty-format working with constructor.name calls. |
092329f to
74ac554
Compare
|
@jmeistrich this also needs your attention again. |
|
@jmeistrich weekly reminder |
|
@jmeistrich gentle weekly reminder |
|
@codeart1st I pushed a small commit to tweak that constructor behavior so if the wrapped object has a constructor it returns that before creating a new one. Does that seem reasonable to you? |
|
@jmeistrich looks very reasonable to me. Nice addition. |
In unit tests without .peek() or .get(), we encountered many exceptions due to the behavior of vitest and chai when iterating over objects for equality checks and error printing. Making this more resilient without throwing exceptions seems like a useful improvement for me.
c3b4be0 to
e734f6e
Compare
In unit tests without .peek() or .get(), we encountered many exceptions due to the behavior of vitest and chai when iterating over objects for equality checks and error printing. Making this more resilient without throwing exceptions seems like a useful improvement for me.