xstate
xstate copied to clipboard
[docs] Verify TypeScript examples in docs
This is an addendum to #2234. Sorry that I have the original commits in this PR, couldn't figure out a better way to do it.
Verify and fix all TypeScript examples in docs
This PR also introduces yarn verifyTypescriptExamples
command that checks every piece of TypeScript code in the docs and runs it against the TS compiler.
⚠️ No Changeset found
Latest commit: e0e0c50bead9ff47084a1d63b1992b919d8fa15d
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
https://github.com/davidkpiano/xstate/pull/2235/commits/3d86fed43b46a2a7caf5e2011e3198736af6bbd1 and https://github.com/davidkpiano/xstate/pull/2235/commits/433887fd24ad9219c8da528fbaf685e35aa9a241 fix the errors the script found.
Now the output is:
➜ xstate git:(doc/fix-typescript-guide) yarn verifyTypescriptExamples
yarn run v1.22.10
$ find packages docs -iname '*.md' -not -path '*/node_modules/*' -not -path '*/.vuepress/*' -not -iname 'CHANGELOG.md' -exec scripts/verifyTypescript {} \;
docs/guides/context.md:389.ts (396,21): Cannot find name 'CounterContext'.
docs/guides/context.md:389.ts (396,37): Cannot find name 'CounterEvent'.
Done in 17.15s.
This is ready for review.
Should most (all?) example code be rewritten in TS?
I did that for context.md
(not published) but then stopped because I'm not sure if this is desirable - I mean, it is for me, but not sure in general?
I'll publish the context.md
update if everyone agrees we should move towards using TS for all example code.
Should most (all?) example code be rewritten in TS?
@VanTanev I think so because:
- types provide context for the examples even if you don't end up using them yourself
- it's significantly easier to read ts and understand which parts to remove if using js than it is to read js and mentally keep track of what all of the things are supposed to be
It'd also be fairly easy to hook up a remark
plugin like remark-typescript
to add the ability to switch between them if we wanted or something.
OK, added the condext.md
TS rewrite!
I also added the ability to verify JS, but have not fixed the errors that show up:
$ VERIFY_JS=1 yarn verifyCodeExamples
yarn run v1.22.10
$ find packages docs -iname '*.md' -not -path '*/node_modules/*' -not -path '*/.vuepress/*' -not -iname 'CHANGELOG.md' -exec
...
Found 686 issues
error Command failed with exit code 1
q: would https://github.com/shikijs/twoslash provide similar functionality (and more)? I don't intend to block this PR - it is useful on its own and we don't want to pursue integrating twoslash
right away. Just thought that maybe you have already looked into it - because I didn't have much time to do that myself - and that maybe you would already have some opinions on it that could be shared.
q: would https://github.com/shikijs/twoslash provide similar functionality (and more)?
It does!
However, it wasn't released at the time I wrote this PR, and integration seems non-trivial.
Do you think the PR is worth merging as is, or should I try to instead integrate twoslash
?
This is ready for re-review. I think it would be useful to merge as is and consider moving to twoslash
in the future when someone has the time to do that.
My only question now is how approachable these examples are going to be to those using JavaScript, and not TypeScript? Should we find a way to allow developers to toggle between the two?
Let's leave that for a separate PR. There are fixes in here that will be useful to xstate users today. We can strive to make it even better tomorrow.
My only question now is how approachable these examples are going to be to those using JavaScript, and not TypeScript? Should we find a way to allow developers to toggle between the two?
Let's leave that for a separate PR. There are fixes in here that will be useful to xstate users today. We can strive to make it even better tomorrow.
It will help some users but will confuse others, see here: https://twitter.com/DavidKPiano/status/1404806818832977927
I want to be cautious about merging this unless we can satisfy both JS and TS users, and I think the best way to do it would be to show JS and TS together, via something like this: https://github.com/phryneas/remark-typescript-tools
We cannot merge this to main
as-is unless we have both languages.
OK. I don't have the bandwidth to do both right now, not sure when I will.
OK. I don't have the bandwidth to do both right now, not sure when I will.
We can take this as a starting point and work with it; thanks so much for the work on this so far!