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

Fix: make optional chaining expression work #1684

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

Conversation

andreabergia
Copy link
Contributor

@andreabergia andreabergia commented Oct 7, 2024

I.e. makes this work:

o = {a: true};
o?.['a']

Note that the spec says that the expression on the right-hand side should not be evaluated if the left-hand side object is null or undefined.

Follow-up from #1593

I.e. code such as:

```js
o = {a: true};
o?.['a']
```

Note that the spec says that the expression on the right-hand side
should _not_ be evaluated if the left-hand side object is null or
undefined.
@andreabergia andreabergia force-pushed the optional-chaining-expressions-2 branch from 28c7c84 to 52a1ce2 Compare October 7, 2024 11:52
@rbri
Copy link
Collaborator

rbri commented Oct 7, 2024

Great!

@p-bakker
Copy link
Collaborator

p-bakker commented Oct 7, 2024

I guess you're saying that the previously merged optional chaining operator support didn't properly supported computed properties?

Wrt to your 'note': think you filled #1600 for that already, no?

@andreabergia
Copy link
Contributor Author

I guess you're saying that the previously merged optional chaining operator support didn't properly supported computed properties?

It didn't support them at all - it didn't even parse the syntax 🙂

Wrt to your 'note': think you filled #1600 for that already, no?

Yeah, but I have done something smaller in scope in this PR: I've just made this new implementation work correctly per the spec. I haven't fixed the general problem because, honestly, I can't imagine how to do it without having a big performance impact.

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.

3 participants