#51 Twoslash vue support#52
Conversation
🦋 Changeset detectedLatest commit: fd96b32 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for expressive-code-twoslash ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
commit: |
Adammatthiesen
left a comment
Adammatthiesen
left a comment
There was a problem hiding this comment.
I like the simplicity overall, only one nit-pick
packages/twoslash/src/index.ts
Outdated
| const isVue = codeBlock.language === 'vue'; | ||
| const selectedTwoslasher = isVue ? twoslasherVue : twoslasher; |
There was a problem hiding this comment.
I like the concept of everything going on here... but i almost wonder if we should do a switch block or something along those lines... since there could be other language plugins added in the future 🤔
Maybe something like this? (To be clear... a pipe-dream of mine is to make a Astro Twoslasher 😅)
| const isVue = codeBlock.language === 'vue'; | |
| const selectedTwoslasher = isVue ? twoslasherVue : twoslasher; | |
| let selectedTwoslasher = twoslasher; | |
| switch (codeBlock.language) { | |
| case "vue": { | |
| selectedTwoslasher = twoslasherVue; | |
| break; | |
| } | |
| default: { | |
| selectedTwoslasher = twoslasher; | |
| break; | |
| } | |
| } |
There was a problem hiding this comment.
That's fair.
I'll add a comment on the changes I pushed :)
This is better prepared for future additions of supported languages.
| const availableTwoSlashers: Record<string, TwoslashInstance> = { | ||
| 'default': createTwoslasher({ | ||
| ...twoslashOptions, | ||
| }), | ||
| 'vue': createTwoslasherVue({ | ||
| ...twoslashOptions, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
In my mind, this will make it easy to extend the code in one place: you add a key and a TwoslashInstance and you're good to go.
| // Select the appropriate twoslasher based on language | ||
| const isVue = codeBlock.language === 'vue'; | ||
| const selectedTwoslasher = isVue ? twoslasherVue : twoslasher; | ||
| const selectedTwoslasher = availableTwoSlashers[codeBlock.language] ?? availableTwoSlashers.default; |
There was a problem hiding this comment.
Then no need for a switch case here, since this line already picks up the matching language's TwoslashInstance.
|
I'll merge this soon, i'm also going to have to update the CI to allow OIDC NPM publishing... as i haven't done that for this repo yet... that way i'll be able to release the new version |
|
Merging this now, but am prepping to move this repo to the |
2 participants
Description
My take on #51. I decided to give it a shot myself by following the comments you shared in the issue.
I also added a docs page, not sure if that would be the best place, but better than nothing I guess 😅