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 connect API #27

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

djlauk
Copy link
Contributor

@djlauk djlauk commented Oct 7, 2020

A first shot at implementing the changes required for addressing issue #26

I am not sure, if making Svelte a peerDependency is the right move. There are pros and cons to it, as per this article on peerDependencies

What we need is a way of expressing these "dependencies" between plugins and their host package. Some way of saying, "I only work when plugged in to version 1.2.x of my host package, so if you install me, be sure that it's alongside a compatible host." We call this relationship a peer dependency.

This ☝️ is not the case here, as Rdx will work just fine without svelte. But then in that same article it is written:

Most plugins never actually depend on their host package, i.e. grunt plugins never do require("grunt") [...]
Even for plugins that do have such direct dependencies, probably due to the host package supplying utility APIs, specifying the dependency in the plugin's package.json would result in a dependency tree with multiple copies of the host package—not what you want.

If merged:
closes #26

@CaptainCodeman
Copy link
Owner

Thanks. I think it makes sense to split the connect into components / svelte as discussed in slack and a peer dependency feels right for people who aren't using svelte.

I'd still like to work with svelte more before adding this to rdx though - the connect layer is pretty small but I want to validate the patterns a bit more first. For instance, instead of doing the connect in each component, it seems to work well to just create and import a connected 'state' and also use that to access the dispatch methods as well (which is why I originally added it instead of it just returning the subscribe method).

@djlauk
Copy link
Contributor Author

djlauk commented Oct 12, 2020

🤔
Indeed the mapState + mapEvents combo leans towards the "classical" React / Redux integration heritage. While I have not done a lot of coding with Svelte yet, I do agree it does not really match the reactive (forward referencing) nature of Svelte. Maybe a single module like this would be more "svelte-ish"? Something like that, maybe:

// src/state/state-store.ts
import { connect } from '@captaincodeman/rdx/svelte'
import { store } from './store'

export const stateStore = connect(store)

And then use it like this:

<!-- src/ui/Foo.svelte -->
<script lang="ts">
  import { stateStore } from '../state/state-store'
  import { fooSelectors } from '../state/models/foo'

  $: numberOfFoos = fooSelectors.fooCount($stateStore)
</script>

<h1>There {numberOfFoos === 1 ? 'is' : 'are'} {numberOfFoos} {numberOfFoos === 1 ? 'foo' : 'foosens'}</h1>
<button on:click={stateStore.dispatch.foo.addFoo()}>Add a foo</button>

The one thing I really dislike about the snippets I just wrote is, that both Rdx and Svelte use the term "store" and it means something different. That does feel like an invitation for misunderstandings (and difficult to write or very wordy docs).

Then, again, if it was export const state = ..., then the client code would read state.dispatch.foo.addFoo(), while for a class based (web component based) UI it would be store.dispatch.foo.addFoo(). I do not like that one time dispatch is on the (Rdx) store, and once it's on some intermediary which might be mistaken for store.state (where store is the Rdx store). I think, after all, I'd prefer, to just import dispatch from src/state/store.ts, as seems to be idiomatic for Rdx (per the docs).

So, to sum it up: Maybe best just to provide read access to the Rdx store's state through a custom Svelte store, and have dispatch exported as it is from src/state/store.ts (i.e. limit the amount of "magic")?

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.

connect API for Svelte
2 participants