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

"Cannot read property 'subscribe' of undefined" when store creation is delayed #2181

Closed
EmilTholin opened this issue Mar 8, 2019 · 5 comments · Fixed by #4304
Closed

"Cannot read property 'subscribe' of undefined" when store creation is delayed #2181

EmilTholin opened this issue Mar 8, 2019 · 5 comments · Fixed by #4304

Comments

@EmilTholin
Copy link
Member

EmilTholin commented Mar 8, 2019

If you don't assign a store to a variable directly on initialisation you get the error "Cannot read property 'subscribe' of undefined" in [email protected].

This is a bit of a contrived example, but demonstrates the problem (REPL):

<!-- Foo.svelte -->
<script>
  import { writable } from 'svelte/store'
	
  export let prop = null
  // Assigning a store to this variable further down gives rise to error
  // "Cannot read property 'subscribe' of undefined"
  let label;
  // This works
  // let label = prop === null ? writable('Foo') : writable(prop)

  if (prop === null) {
    label = writable('Foo')
  } else {
    label = writable(prop)
  }
</script> 

<div>{$label}</div>

This worked back in beta.3.

@Conduitry
Copy link
Member

Simpler example:

<script>
	import { writable } from 'svelte/store';
	let foo;
	foo = writable(42);
</script>

{$foo}

I'm not sure what we want to do about this. One of the several things that changed in beta 4 was to subscribe to stores right away after they're declared. This is generally useful but does result in an error in this case. If foo is not a store, should the generated $$subscribe_foo function check for that and not do anything in that case? Is that worth the bytes? Is it unreasonable to say that if someone refers to $foo anywhere in their component, that foo is assumed to always be a store?

@LaughingBubba
Copy link

Hi I'm getting the same problem but only when I run the code in https://codesandbox.io , When I run the code locally no problems. Go figure.

@hexrcs
Copy link

hexrcs commented Nov 26, 2019

I ran into this issue recently and wasn't able to figure out a way to work around this. Are there any more details on why the creation is delayed (batched for performance?) or when exactly the stores are created?

Edit: in my case, I placed the stores inside a separate .js file but I think it's basically the same thing as here.

@pushkine
Copy link
Contributor

pushkine commented Dec 12, 2019

Everytime I got that error the debugger pointed me to the wrong store, the real one is named on line 6 at $$subscribe_storeName

@Conduitry
Copy link
Member

I don't remember what the autosubscription implementation looked like back when this issue was first opened, but the way it is now would actually lend itself pretty easily to supporting this.

diff --git a/src/runtime/internal/utils.ts b/src/runtime/internal/utils.ts
index c7722e1a0..4ba7e18c1 100644
--- a/src/runtime/internal/utils.ts
+++ b/src/runtime/internal/utils.ts
@@ -43,12 +43,15 @@ export function not_equal(a, b) {
 }
 
 export function validate_store(store, name) {
-	if (!store || typeof store.subscribe !== 'function') {
+	if (store != null && typeof store.subscribe !== 'function') {
 		throw new Error(`'${name}' is not a store with a 'subscribe' method`);
 	}
 }
 
 export function subscribe(store, ...callbacks) {
+	if (store == null) {
+		return noop;
+	}
 	const unsub = store.subscribe(...callbacks);
 	return unsub.unsubscribe ? () => unsub.unsubscribe() : unsub;
 }

All subscriptions now go through this one function (to massage the response from RxJS observables), so we can also use this opportunity to make subscribing to a nullish store (and unsubscribing from it later) a no-op if so desired.

@Conduitry Conduitry removed the awaiting submitter needs a reproduction, or clarification label Jan 22, 2020
Conduitry added a commit to Conduitry/sveltejs_svelte that referenced this issue Jan 22, 2020
Rich-Harris pushed a commit that referenced this issue Jan 23, 2020
* make autosubscribing to a nullish store a no-op (#2181)

* update changelog

* add test
jesseskinner pushed a commit to jesseskinner/svelte that referenced this issue Feb 27, 2020
* make autosubscribing to a nullish store a no-op (sveltejs#2181)

* update changelog

* add test
taylorzane pushed a commit to taylorzane/svelte that referenced this issue Dec 17, 2020
* make autosubscribing to a nullish store a no-op (sveltejs#2181)

* update changelog

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

Successfully merging a pull request may close this issue.

5 participants