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

svelte/store derived doesn't handle RxJS observables #4298

Closed
Conduitry opened this issue Jan 21, 2020 · 4 comments · Fixed by #4300
Closed

svelte/store derived doesn't handle RxJS observables #4298

Conduitry opened this issue Jan 21, 2020 · 4 comments · Fixed by #4300
Labels

Comments

@Conduitry
Copy link
Member

Describe the bug
The derived implementation doesn't handle RxJS Observables like other autosubscription stuff does.

Logs

TypeError: "fn is not a function"

To Reproduce

<script>
	import { of } from 'rxjs';
	import { derived } from 'svelte/store';
	const store1 = of('foo');
	const store2 = derived(store1, _ => _);
	store2.subscribe()();
</script>

Expected behavior
Subscribing to and unsubscribing from a store derived from an observable should work. In particular, this means the 'look to see whether there's an unsubscribe method to call instead' logic we use elsewhere should also be use in the derived implementation.

Information about your Svelte project:

  • Svelte 3.17.2
  • REPL

Severity
Probably worth the bytes it would take to fix.

Additional context
A question came up in chat about using Observables in Svelte. I don't know whether the gap this issue describes is part of what was wrong, but the question did lead me to look into this.

We're already shipping a few extra bytes to everyone for RxJS support whether they're using it or not. It would be nice to be able to use the same helper in derived as we're already using in every Svelte project that uses autosubscription, but that might not be possible because of the two-argument subscribe() calls for diamond dependency stuff.

@vipero07
Copy link

This seems to work fine in REPL (though maybe I misunderstood your intent)

https://svelte.dev/repl/ce3b3cf61ea942c9bc3f67667d37188a?version=3.17.2

@Conduitry
Copy link
Member Author

That works fine because the component with the autosubscription is never destroyed. When we try to remove the last subscription from a derived that was based on an RxJS observable is when the exception is thrown, because derived expects to be able to call the return value from the observable's .subscribe() method as a function, when it is in fact an object with an .unsubscribe() method.

Conduitry added a commit to Conduitry/svelte that referenced this issue Jan 22, 2020
@Conduitry
Copy link
Member Author

This doesn't look like it was actually fixed for some reason, reopening.

@Conduitry Conduitry reopened this Jan 23, 2020
@Conduitry
Copy link
Member Author

Never mind, this is fixed in 3.17.3. The OP was a bad repro. store2.subscribe()() is invalid because it attempts to use undefined as a subscription callback. Here's a better test:

<script>
	import { of } from 'rxjs';
	import { derived } from 'svelte/store';
	const store1 = of('foo');
	const store2 = derived(store1, _ => _);
	store2.subscribe(() => {})();
</script>

This breaks in 3.17.2 and works in 3.17.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants