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(fast-element-1): some template bindings not properly being marked as volatile #6966

Open
m-akinc opened this issue May 21, 2024 · 0 comments
Labels
status:needs-investigation Needs additional investigation status:under-consideration Issue is being reviewed by the team.

Comments

@m-akinc
Copy link
Contributor

m-akinc commented May 21, 2024

🐛 Bug Report

Some bindings that should be marked as volatile are not, so changes may not be properly detected.

  1. While there is code to detect some conditionals in binding expressions, it does not trigger on ??. This means a binding expression such as x => x.foo ?? x.bar might miss changes to bar.
  2. Aggregate bindings are never marked as volatile due to a bug in the logic. The HTMLBindingDirective constructor tests the stringified binding function for volatility, rather than the parsed parts of the aggregate expression.

💻 Repro or Code Sample

The following binding expressions will not reliably detect changes:

  • my-attr="${x => x.foo ?? x.bar} because nullish coalescing does not match the regex
  • my-attr="foo ${x => x.bar && x.baz} because aggregate bindings are never marked volatile
  • my-attr="foo ${x => x.bar || x.baz} because aggregate bindings are never marked volatile

🤔 Expected Behavior

Attributes should always be updated when a value in the binding expression changes.

😯 Current Behavior

Attributes are not always updated when a value in the binding expression changes.

💁 Possible Solution

🔦 Context

Discovered during resolution of ni/nimble#1839

@m-akinc m-akinc added the status:triage New Issue - needs triage label May 21, 2024
@chrisdholt chrisdholt added status:needs-investigation Needs additional investigation status:under-consideration Issue is being reviewed by the team. and removed status:triage New Issue - needs triage labels May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:needs-investigation Needs additional investigation status:under-consideration Issue is being reviewed by the team.
Projects
None yet
Development

No branches or pull requests

2 participants