-
Notifications
You must be signed in to change notification settings - Fork 19
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
Basic Todos Example no longer works as of utils version 0.9.7 #310
Comments
Hey @d-krause thank you for flagging this up and coming up with a solution. The tutorial was affected by the changes on I've already fixed the sandbox and a PR for updating the docs is already on the way re-rxjs/react-rxjs.org#59 Some clarification though, on the points you said that would need to be changed:
It's not really needed. The observable returned by
This is the place that had to be updated. But now If you need to get a set of the keys at any given moment, utils also exports const [useTodos, todos$] = bind(
keys$.pipe(
toKeySet(),
map(set => Array.from(set))
)
);
Same reason as above, 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. |
sorry, but did you just introduce a breaking change with a PATCH version? |
@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 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 |
@voliva Thanks for clarifying and a migration tip! Really enjoying the lib. |
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.
.pipe( startWith( { payload: { id: 0, text: '' }, type: 'delete' } ));
const [useTodos, todos$] = bind(keys$);
would need to becomeconst [useTodos, todos$] = bind(keys$.pipe(map(x => Array.from(x.keys))));
if(i === 0)
condition would need to be removed from thegroupedObservables$
map function call in thepartitionByKey
return value.Example patch for change #3 using conditional breakpoint in codesandbox
The text was updated successfully, but these errors were encountered: