Don't cache parsed value of selector(All) property types #5521
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR addresses a small regression introduced with #5474. The
selector
andselectorAll
property types have a parse function that can be considered impure, i.e. the output depends on more than just the value being parsed. The current state of the DOM at the time of parsing factors in as well.#5474 shifted the moment properties were parsed and cached in case of entities that hadn't loaded yet. This works fine as long as the selector can already match the relevant entities at that time. @arpu ran into a situation where a small HTML template was constructed in full before being inserted into the scene. These could contain selectors that would refer to elements in the same snippet. For illustration, consider the following snippet being constructed dynamically post scene-load:
To resolve this, the PR adds a
isCacheable
property to the property types which signals if the parsed value may be cached or not. This prevents theselector/selectorAll
property types from being prematurely parsed and cached, better matching the old behaviour.@arpu Could you verify that this solves the issue for you?
Changes proposed:
isCacheable
flag to property typesselector
andselectorAll