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

Throw an error if the attribute is truly undefined #1215

Merged
merged 13 commits into from
Sep 21, 2021
Merged

Conversation

samscott89
Copy link
Member

Differentiates between {x: undefined}.x = y and {}.x = y. The former is fine with y = undefined, the latter raises an error.

Fixes #749

Discussed this approach with @gj a few weeks back. Check for the attribute with "x" in d.

@samscott89 samscott89 requested a review from gj September 21, 2021 13:45
@github-actions
Copy link

github-actions bot commented Sep 21, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@samscott89
Copy link
Member Author

@gj what do you think? This would be a breaking change. Do you think anyone would depend on the old behaviour?

languages/js/src/Query.ts Show resolved Hide resolved
languages/js/src/Query.ts Show resolved Hide resolved
@gj
Copy link
Member

gj commented Sep 21, 2021

@gj what do you think? This would be a breaking change. Do you think anyone would depend on the old behaviour?

https://xkcd.com/1172/

but yeah I think it's a good change that we should make

languages/js/src/Polar.test.ts Outdated Show resolved Hide resolved
languages/js/src/Polar.test.ts Outdated Show resolved Hide resolved
samscott89 and others added 5 commits September 21, 2021 11:33
Co-authored-by: Gabe Jackson <17556281+gj@users.noreply.github.com>
Co-authored-by: Gabe Jackson <17556281+gj@users.noreply.github.com>
@gj gj merged commit 02787e8 into main Sep 21, 2021
@gj gj deleted the sam/js-undefined-check branch September 21, 2021 17:15
@github-actions github-actions bot locked and limited conversation to collaborators Sep 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-existent attributes still match in a rule head specializer in Node.JS library
2 participants