Skip to content

Conversation

@stellarhoof
Copy link
Member

@stellarhoof stellarhoof commented Sep 25, 2025

Fix onUpdateByOthers to run also when a type's reactors is set to all. Currently only reactors set to other cause this function to run.

@stellarhoof stellarhoof self-assigned this Sep 25, 2025
@changeset-bot
Copy link

changeset-bot bot commented Sep 25, 2025

🦋 Changeset detected

Latest commit: d053af2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
contexture-client Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@stellarhoof stellarhoof marked this pull request as draft September 25, 2025 15:53
@stellarhoof stellarhoof marked this pull request as ready for review September 25, 2025 16:39
@stellarhoof stellarhoof merged commit 6331c8a into main Sep 25, 2025
3 checks passed
@stellarhoof stellarhoof deleted the feature/trigger-onUpdateByOthers-on-reactors-all branch September 25, 2025 16:40
@github-actions
Copy link
Contributor

Coverage after merging feature/trigger-onUpdateByOthers-on-reactors-all into main will be

99.59%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
packages/client/src
   exampleTypes.js95.25%85.71%40%98.50%109, 123, 126–132
   index.js98.90%93.85%100%100%216, 218, 289, 73
   lens.js100%100%100%100%
   listeners.js100%100%100%100%
   mockService.js95.65%83.33%100%100%22, 30
   node.js97.48%96%87.50%98.84%72, 76
   reactors.js99.12%100%90.91%100%
   serialize.js100%100%100%100%
   subquery.js98.80%88.89%100%100%66
   traversals.js100%100%100%100%
   types.js92.59%83.33%100%95.24%12–13
   validation.js78.13%80%100%77.78%20–26
packages/client/src/actions
   index.js100%100%100%100%
   wrap.js98.33%85.71%100%100%41
packages/client/src/exampleTypes
   pivot.js83.18%74.29%87.50%84.07%10–13, 138–139, 14, 140–149, 15, 150–160, 167–168, 21–22, 227, 23, 243, 257–263, 48, 53–57, 8–9, 9
packages/client/src/util
   futil.js67.31%100%57.14%65.85%18–19, 24–28, 31, 36–41
   tree.js100%100%100%100%
packages/export/src
   csv.js94.74%80%50%98%26, 47
   excel.js94.59%76.92%50%98.94%13, 38, 52, 91
   index.js0%0%0%0%1, 1–4, 6
   utils.js100%100%100%100%
packages/export/src/nodes
   fieldValuesGroupStats.js75%70%66.67%76.74%25, 29, 34–43, 9
   index.js0%0%0%0%1, 1–5
   pivot.js98.40%82.35%100%100%52, 56, 89
   results.js100%100%100%100%
   terms_stats.js100%100%100%100%
packages/provider-elasticsearch/src
   compat.js100%100%100%100%
   index.js79.26%46.43%80%85.33%100, 104, 104, 116–121, 123, 129, 13, 130–139, 14, 30, 61–67, 72–73, 79, 81, 88–89
   schema.js88.39%92.86%80%88.17%53–56, 86–93
   types.js100%100%100%100%
packages/provider-elasticsearch/src/example-types
   index.js100%100%100%100%
   schemaMapping.js100%100%100%100%
   testUtils.js98.88%92.31%100%100%12
packages/provider-elasticsearch/src/example-types/filters
   bool.js100%100%100%100%
   date.js87.18%66.67%50%91.18%10–11, 19, 9
   dateRangeFacet.js90.35%100%80%89.90%90–99
   exists.js100%100%100%100%
   facet.js91.47%76.47%66.67%94.50%39–40, 65–72
   geo.js100%100%100%100%
   number.js52.05%100%50%50%22–29, 33–56
   query.js100%100%100%100%
   step.js82.05%100%60%83.87%19–23
   tagsQuery.js86.98%90.63%85.71%86.36%111–128, 140, 149–153, 159–161
   tagsText.js100%100%100%100%
   text.js96.58%86.67%100%100%11, 22, 40, 71
packages/provider-elasticsearch/src/example-types/legacy
   cardinality.js93.33%100%50%100%
   dateHistogram.js95.45%100%50%100%
   esTwoLevelAggregation.js92.11%86.67%66.67%93.75%34–36, 83–87
   matchStats.js97.06%100%50%100%
   rangeStats.js100%100%100%100%
   smartIntervalHistogram.js97.44%100%50%100%
   statistical.js90.91%100%50%100%
   terms_stats.js97.73%100%66.67%100%
packages/provider-elasticsearch/src/example-types/metricGroups
   dateIntervalGroupStats.js100%100%100%100%
   dateRangesGroupStats.js98.28%100%75%100%
   fieldValuePartitionGroupStats.js87.76%100%40%92.31%34–36
   fieldValuesDelta.js97.44%100%66.67%100%
   fieldValuesGroupStats.js99.07%100%83.33%100%
   groupStatUtils.js86.05%100%75%85.71%20–24
   numberIntervalGroupStats.js100%100%100%100%
   numberRangesGroupStats.js97.87%100%66.67%100%
   percentilesGroupStats.js82.46%75%50%85.71%16–18, 24–26, 43–44
   pivot.js85.73%89.77%71.43%85.81%107, 151–152, 159, 166, 408–410, 460–470, 473, 478, 480, 483, 486, 534–535, 563–618, 75–81, 84–91
   stats.js91.30%100%50%100%
   tagsQueryGroupStats.js100%100%100%100%
packages/provider-elasticsearch/src/example-types/metricGroups/pivotData
   columnResponse.js100%100%100%100%
   columnResult.js100%100%100%100%
   pivotResponse.js100%100%100%100%
   pivotResponseWithFilteredFieldValueGroup.js100%100%100%100%
packages/provider-elasticsearch/src/example-types/results
   index.js20.93%100%0%21.95%10–39, 8–9
packages/provider-elasticsearch/src/example-types/results/highlighting
   request.js95.27%93.75%100%95.41%200, 200–208, 40–42
   response.js100%100%100%100%
   search.js21.05%100%0%21.43%20–63
   testSchema.js100%100%100%100%
   util.js100%100%100%100%
packages/provider-elasticsearch/src/schema-data
   aliases.js100%100%100%100%
   mapping-with-non-objects.js100%100%100%100%
   mapping-with-types.js100%100%100%100%
   mapping-without-types.js100%100%100%100%
   schema-with-types.js100%100%100%100%
   schema-without-types.js100%100%100%100%
packages/provider-elasticsearch/src/utils
   elasticDSL.js100%100%100%100%
   fields.js100%100%100%100%
   futil.js99.39%100%92.86%100%
   luceneQueryUtils.js100%100%100%100%
   regex.js100%100%100%100%
   results.js100%100%100%100%
   smartInterval.js83.33%33.33%100%92.86%11, 11, 9
packages/provider-mongo/src
   index.js93.33%71.43%83.33%96.77%10, 33, 45, 9
   types.js100%100%100%100%
packages/provider-mongo/src/example-types
   bool.js100%100%100%100%
   date.js98%90.91%100%100%23
   dateHistogram.js95.65%62.50%100%100%19, 25, 25
   exists.js100%100%100%100%
   facet.js99.30%95.74%100%100%123–124
   index.js100%100%100%100%
   mongoId.js100%100%100%100%
   number.js100%100%100%100%
   results.js97.74%92.31%100%99.01%180, 59, 61, 64–66
   statistical.js100%100%100%100%
   tagsText.js100%100%100%100%
   termsStats.js100%100%100%100%
   text.js100%100%100%100%
packages/server/src
   dataStrategies.js85.71%100%75%85%28–36
   exampleTypes.js97.10%83.33%100%100%6, 9
   index.js97.53%100%100%96.97%47–48
   utils.js88.37%93.75%100%86.02%44–49, 63–71
packages/server/src/__data__
   imdb.js100%100%100%100%
packages/server/src/provider-debug
   index.js100%100%100%100%
packages/server/src/provider-memory
   date.js96.67%100%60%100%
   exampleTypes.js91.61%91.67%81.25%93.20%100, 16–17, 50, 64, 96–99
   index.js95%66.67%100%100%10
   results.js95.24%50%100%100%14
packages/util/src
   dateUtil.js0%0%0%0%1, 1, 10–19, 2, 20–29, 3, 30–39, 4, 40–49, 5, 50–59, 6, 60–69, 7, 70–74, 8–9
   keywordGenerations.js100%100%100%100%
packages/util/src/exampleTypes
   tagsQuery.js0%0%0%0%1, 1, 10–12, 2–9

@daedalus28
Copy link
Collaborator

Just reiterating that onUpdateByOthers is intended not to react to self updates from an all reactor - it's about reacting to updates by others. I'm pretty sure there was a unit test asserting this behavior at some point. This change is likely to introduce subtle bugs that will be hard to track down (or even notice at first).

Conceptually, onUpdateByOthers gives nodes a way to react to things that happen to it that it normally has no control over. By changing the semantics, that no longer exists. It also makes the name confusing because it no longer does what it says - especially when there are existing options in the API that overlap with this change.

If none of the existing hooks work for you, my suggestion would be to introduce a new lifecycle hook to avoid breaking anything existing. Alternatively, rename this one so its name matches its semantics. That would force you to change everything using it and would help surface potential bugs before they happen.

But of course this is just a suggestion. Feel free to ignore it entirely.

@stellarhoof
Copy link
Member Author

Just reiterating that onUpdateByOthers is intended not to react to self updates from an all reactor

It still doesn't do this. The self node is filtered out so onUpdateByOthers does not get called on it.

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