feat(dev-tools): use new sync stack #1284
Conversation
holic
commented
Aug 11, 2023
•
holic
commented
- change dev tools to mount with arguments rather than magical globals
- update dev tools to use new sync stack data
- add basic recs support
🦋 Changeset detectedLatest commit: 253e08a The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| abi: IWorld__factory.abi, | ||
| publicClient, | ||
| walletClient: burnerWalletClient, | ||
| onWrite: (write) => write$.next(write), |
There was a problem hiding this comment.
wondering if we wanna export something like write$ from createContract rather than the onWrite callback where user land has to convert it to an observable
downside is that common will need rxjs as a dep, which is quite heavy
There was a problem hiding this comment.
i like the lightweight onWrite callback
holic
changed the title
| worldAbi: result.network.worldContract.abi, | ||
| write$: result.network.write$, | ||
| recsWorld: result.network.world, | ||
| }); |
There was a problem hiding this comment.
looks fine, maybe add a comment linking to https://vitejs.dev/guide/env-and-mode.html near import.meta.env.DEV?
.changeset/soft-dryers-invite.md
Outdated
There was a problem hiding this comment.
Multiple changesets in one PR might look a little odd in the global changelog because the same headline (the PR title) will be used multiple times. Would it be feasible to split these three changes to three PRs or are they too intertwined?
There was a problem hiding this comment.
Multiple changesets in one PR might look a little odd in the global changelog because the same headline (the PR title)
I think we should instead adapt how we generate the changelog to not emphasize the PR titles so much (since the changeset should capture what changed). I really like being able to encapsulate things in one PR yet publish multiple changesets.
There was a problem hiding this comment.
Do you have a proposal for how to separate changesets in the global changelog? One alternative i see is manually enforcing a h2 headline in the changeset, but it might cause annoying formatting issues in case we miss it somewhere
There was a problem hiding this comment.
I quite like how viem looks: https://github.com/wagmi-dev/viem/blob/main/CHANGELOG.md
There was a problem hiding this comment.
Yeah that's the changesets default for a single package. Since our changesets tend to be more verbose and include context on the change and how to integrate it, the list format felt a little less clear. We could use the commit hash instead of the PR title though, or just accept the same PR title being mentioned multiple times.
There was a problem hiding this comment.
gonna land this and revisit the changelog and see if there's a way to make this render nicely
| abi: IWorld__factory.abi, | ||
| publicClient, | ||
| walletClient: burnerWalletClient, | ||
| onWrite: (write) => write$.next(write), |
There was a problem hiding this comment.
i like the lightweight onWrite callback
