Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

fix(core): handle setting falsy value in proxy providers #118

Merged
merged 2 commits into from
Jan 31, 2024
Merged

fix(core): handle setting falsy value in proxy providers #118

merged 2 commits into from
Jan 31, 2024

Conversation

alexandre-abrioux
Copy link
Contributor

This PR fixes setting a falsy value on a proxyfied object. Without the fix, the test fails with the following error when running proxy.booleanTest = false:

'set' on proxy: trap returned falsish for property 'booleanTest'

This behavior is documented here on MDN:

If the set() method returns false, and the assignment happened in strict-mode code, a TypeError will be thrown.

@alexandre-abrioux @jerrylaloan
fix(proxy-provider-manager): handle setting falsy value
846c678 
Co-authored-by: Jerry Laloan <jerrylaloan@users.noreply.github.com>
Papooch
Papooch requested changes Jan 31, 2024
View reviewed changes
Copy link
Owner

@Papooch Papooch left a comment

Choose a reason for hiding this comment

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

Great catch, thank you! But please move the test case to packages/core/test/proxy-providers/proxy-providers.spec.ts to a new top-level describe block called "edge cases".

@alexandre-abrioux
Copy link
Contributor Author

@Papooch Thanks! Done in 1623c21

@Papooch Papooch changed the title fix(proxy-provider-manager): handle setting falsy value fix(core): handle setting falsy value in proxy providers Jan 31, 2024
@Papooch Papooch merged commit 26737d8 into Papooch:main Jan 31, 2024
2 checks passed
@Papooch
Copy link
Owner

Papooch commented Jan 31, 2024

Released in 4.0.3

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@Papooch Papooch Papooch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alexandre-abrioux @Papooch