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(compiler): handle symbol properties for dynamic values #3895

Conversation

seckardt
Copy link
Contributor

Details

Fix for issue #3892.

Does this pull request introduce a breaking change?

  • ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • ✅ No, it does not introduce an observable change.

GUS work item

@seckardt seckardt requested a review from a team as a code owner December 11, 2023 10:07
@pmdartus
Copy link
Member

All the Karma tests are timing out since @seckardt isn't an LWC collaborator.
Instead of failing with a timeout, it would be great to make those actions require collaborators' manual approval.

@nolanlawson
Copy link
Contributor

@pmdartus I had hoped GitHub Actions had a generic solution for this, but if they do, I haven't found it yet. I just open duplicate PRs to force CI to run. 😞

Copy link
Contributor

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

LGTM overall. Just a few minor suggestions, see comments above.

@seckardt seckardt force-pushed the feature/fix-wire-decorator-symbol-properties branch 2 times, most recently from d779ef8 to 520ce9f Compare December 17, 2023 15:58
@seckardt seckardt force-pushed the feature/fix-wire-decorator-symbol-properties branch from 520ce9f to 710c39a Compare December 17, 2023 16:01
@nolanlawson
Copy link
Contributor

Looking into it further, computed properties actually support all kinds of literals:

{ [1]: 2 }
{ [/foo/]: 2 }
{ [null]: 2 }

We don't need to be pedantic about this, though. I'll open a separate PR to handle these in @wire.

@nolanlawson nolanlawson merged commit 1fff19d into salesforce:master Dec 18, 2023
11 of 14 checks passed
@nolanlawson
Copy link
Contributor

Thanks a lot!

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.

None yet

3 participants