Skip to content

Conversation

@AdrianGonz97
Copy link
Member

@AdrianGonz97 AdrianGonz97 commented Aug 24, 2023

This PR simply adds a note to each of the stores stating that their respective store getter functions must be invoked at the top level of a component.

I also noticed that we weren't exposing the type for each of the stores, so I went ahead and added that with this PR.

Changes

  • Exposed the following type: ToastStore, DrawerStore, and ModalStore
  • Added those newly exposed type to their respective pages in the docs
  • Improved the JSDoc comment for each getter function, such that it specifies that the function must be invoked from the top level of a component:
    img
  • Added this note for each store:
    img

Copying over the original message from Discord here too:

I think this is another topic worth bringing up. Looking into this, the introduction of using contexts to store our toastStore, modalStore, and drawerStore has 1 apparent limitation. In order to retrieve the stores with contexts, we have to call these functions at the top level of components.

For instance, this is valid code:

<script>
  // SomeComponent.svelte
  import { getToastStore } from "@skeletonlabs/skeleton";

  const toastStore = getToastStore();

  toastStore.trigger(...); // this is fine

  // or
  function addToast() {
    toastStore.trigger(...); // this is fine too
  }
</script>

So, in order to use the toastStore (and the others), the store must first be retrieved at the top levels of components to be used.

This means that we can't do something like this:

<script>
  // SomeComponent.svelte
  import { getToastStore } from "@skeletonlabs/skeleton";

  function addToast() {
    // this will throw a "Component Initialization" error as 
    // getting and setting contexts must be done at the top level of components
    const toastStore = getToastStore();
    toastStore.trigger(...)
    
    // this will also throw the same error for the same reason
    getToastStore().trigger(...)
  }
</script>

As we haven't seen many reports of this as part of v2, I figure the usage of calling the getToastStore outside of the top level of a component is fairly uncommon. But for someone who is trying to refactor their code, a reasonable person could assume that they can take getToastStore outside of a component into it's own module, like a toast.ts file.

// toast.ts
import { getToastStore } from "@skeletonlabs/skeleton";

// This will error out for the same reason as the code above.
export function addToast() {
  const toastStore = getToastStore();
  toastStore.trigger(...);
}

If a user want's to create a module for reusable triggers, they'll have to do it like this:

// toast.ts
export function addToast(toastStore, message) {
  toastStore.trigger(message);
}

Where they pass the toastStore to the reusable function. It's a tradeoff, but something that is probably worth documenting

@vercel
Copy link

vercel bot commented Aug 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
skeleton-docs ✅ Ready (Inspect) Visit Preview Aug 25, 2023 4:47pm

@changeset-bot
Copy link

changeset-bot bot commented Aug 24, 2023

⚠️ No Changeset found

Latest commit: 893faac

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@endigo9740
Copy link
Contributor

endigo9740 commented Aug 25, 2023

@AdrianGonz97 your changes look great, but I wasn't really happy with how part of the setup instructions were embedded in the feature example's code tab. I've adjusted the structure so that the critical info like initializing stores and adding the component are part of the full usage information. The code tab now just refers users down to this instruction.

Essentially one single source of truth now, rather than two! You start at the top of the page, read down, then should know everything you need to know to use these features.

@endigo9740 endigo9740 merged commit 0187fdb into v2 Aug 25, 2023
@AdrianGonz97 AdrianGonz97 deleted the docs/gotchas-with-stores branch August 25, 2023 16:56
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.

2 participants