-
Notifications
You must be signed in to change notification settings - Fork 196
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
chore: delete std-client package #1342
Conversation
🦋 Changeset detectedLatest commit: 50ffc62 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since there are still a couple projects using the action queue, i wonder if we should keep this in some other package? can mark it as deprecated
but i feel bad about fully removing it without an alternative option for easily handling optimistic updates. A satisfying replacement could be as simple as a util to define an optimistic update that is removed once the tx is confirmed / all state updates triggered by the tx are reduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Folks can still pin to the old version via npm if they need to stick on the old version for now. And that's an easy to add util, but would be curious to find folks actually using it, how they're using it, and make sure we're providing the right abstractions over it.
Alternatively, we could just move this to the RECS package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can find projects still using it via https://github.com/search?q=createActionSystem+std-client&type=code. It's true that folks can still pin to the old version, but I feel like we should encourage staying up to date with the latest version and make upgrading as easy as possible. The path of least resistance would be moving it to the recs package (and still marking it as deprecated and adding a note for how if no "requirements" are needed there is no need to use the action system, and maybe adding a simple util like
mud/docs/pages/tutorials/emojimon/players-and-movement.mdx
Lines 303 to 315 in ce7125a
const positionId = uuid(); | |
Position.addOverride(positionId, { | |
entity: playerEntity, | |
value: { x, y }, | |
}); | |
try { | |
const tx = await worldSend("move", [x, y]); | |
await awaitStreamValue(txReduced$, (txHash) => txHash === tx.hash); | |
} finally { | |
Position.removeOverride(positionId); | |
} | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm there's still some reliance on ethers and txReduced$
in here 🙈
so I guess createActionSystem
isn't even compatible with the v2 stack as it is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to port most of it over: #1351
But realizing that tests are failing and I'm not sure when they last passed (because we haven't had this package running tests in CI for at least a year).
4af2126
to
5e348a4
Compare
depends on #1339
need to leave the
package.json
behind until we have an answer from changesets/changesets#1215