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

fix deriving from RxJS observables #4300

Merged
merged 5 commits into from
Jan 22, 2020
Merged

Conversation

Conduitry
Copy link
Member

Fixes #4298. I'm not convinced this is the best way to handle this though. Making the subscribe helper accept a ... rest param might not be worth it just to make the code slightly more DRY.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the hesitation about rest args just a semantic thing or possible performance?
JSPerf doesn't show any real measurable difference.

Also, should we be updating the changelog in our PRs?
I'd like to limit the amount of fixup needed for my own.

@@ -48,8 +48,8 @@ export function validate_store(store, name) {
}
}

export function subscribe(store, callback) {
const unsub = store.subscribe(callback);
export function subscribe(store, ...args) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ...callbacks instead of ...args?

@Conduitry Conduitry merged commit 5107ad3 into sveltejs:master Jan 22, 2020
@Conduitry Conduitry deleted the gh-4298 branch January 22, 2020 14:19
jesseskinner pushed a commit to jesseskinner/svelte that referenced this pull request Feb 27, 2020
taylorzane pushed a commit to taylorzane/svelte that referenced this pull request Dec 17, 2020
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.

svelte/store derived doesn't handle RxJS observables
2 participants