Basic Todos Example no longer works as of utils version 0.9.7

[d-krause]

The Basic Todos Example code and sandbox link mentioned in https://react-rxjs.org/docs/tutorial/todos has not worked since the release of @react-rxjs/utils version 0.9.7 on July 12, 2023.

Reverting to a hard version dependency of 0.9.5 for @react-rxjs/utils will allow the example Todo App code to work again. The problem comes from this commit on March 30, 2022 that changed the partitionByKey return value offset 1 from an Observable<K[]> to an Observable<KeyChanges< K >>.

Though the commit was made over one year ago, it was not included in the npm package until the release of version 0.9.7 in July this year.

To recreate and fix the problem, open the sandbox link , see that it does not work, then change the package.json dependency to "@react-rxjs/utils": "0.9.5", and the example app will work again.

To see it work with @react-rxjs/utils version 0.9.7 you will need to revert the package.json change made above and then make the enumerated changes below, with change #3 requiring either a monkey patch or running it all locally.

1. The mergeWithKey call in App.tsx would need a .pipe( startWith( { payload: { id: 0, text: '' }, type: 'delete' } ));
2. The const [useTodos, todos$] = bind(keys$); would need to become const [useTodos, todos$] = bind(keys$.pipe(map(x => Array.from(x.keys))));
3. The if(i === 0) condition would need to be removed from the groupedObservables$ map function call in the partitionByKey return value.

Example patch for change #3 using conditional breakpoint in codesandbox

[voliva]

Hey @d-krause thank you for flagging this up and coming up with a solution. The tutorial was affected by the changes on partitionByKey and we didn't update it.

I've already fixed the sandbox and a PR for updating the docs is already on the way https://github.com/re-rxjs/react-rxjs.org/pull/59

Some clarification though, on the points you said that would need to be changed:

1. The mergeWithKey call in App.tsx would need a .pipe( startWith( { payload: { id: 0, text: '' }, type: 'delete' } ));

It's not really needed. The observable returned by partitionByKey always emits immediately when someone subscribes to it, even if the source stream didn't emit anything. So it shouldn't really need the startWith

2. The const [useTodos, todos$] = bind(keys$); would need to become const [useTodos, todos$] = bind(keys$.pipe(map(x => Array.from(x.keys))));

This is the place that had to be updated. But now partitionByKey doesn't return an observable of the list of keys, but an observable of the keys that were changed (either added or removed).

If you need to get a set of the keys at any given moment, utils also exports toKeySet() as a utility. The code then becomes:

const [useTodos, todos$] = bind(
  keys$.pipe(
    toKeySet(),
    map(set => Array.from(set))
  )
);

3. The if(i === 0) condition would need to be removed from the groupedObservables$ map function call in the partitionByKey return value.

Same reason as above, partitionByKey doesn't emit all the current keys, only those that have changed. The condition i === 0 covers the first point you raised: Every new observer will have an emission of one single change of type "add" with all the keys that are currently active.

This was done for performance reasons, as re-emitting the complete set on every new emission didn't scale well for streams with thousands of events. When composing the streams down, in most cases you're only interested on the values that were changed, and if you need a list it's trivial to build it from the Observable of changes.

[nktka]

sorry, but did you just introduce a breaking change with a PATCH version?

[voliva]

@nktka unfortunately yes. I realised it when fixing the sandbox above.

Human error, sorry. What happened here is that when we did this change #232 we didn't immediately release it, hoping to bundle new changes into a minor version (we're still on 0.x.x). However, this did not happen, and one year later #302 fix came in, which we had to release. I forgot that we didn't release this breaking change as a separate minor version, so it got released as a patch.

It can't be reverted, as trying to revert it would be another violation of semversion. Fortunately, it only affects one function from the utility package, and the "migration path" is simple (use the provided toKeySet() operator). But that's no excuse, we will change the process to avoid this from happening again.

[nktka]

@voliva Thanks for clarifying and a migration tip! Really enjoying the lib.