feat: tutorialkit eject command
#81
Conversation
|
|
|
|
|
||
| const supportedCommands = new Set(['version', 'help', 'create']); | ||
| const supportedCommands = new Set<string>(['version', 'help', 'create', 'eject'] satisfies CLICommand[]); |
There was a problem hiding this comment.
Can't we just type this as Set<CLICommand>?
There was a problem hiding this comment.
We can't because then we get a type error below when trying to do supportedCommands.has(string).
The reason I made that change was to make sure the values are checked against the CLICommand type.
Co-authored-by: Dominic Elm <[email protected]> Co-authored-by: Sam Verschueren <[email protected]>
Deploying tutorialkit-demo-page with
|
| Latest commit: |
0e2b6d8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a52b6d9b.tutorialkit-demo-page.pages.dev |
| Branch Preview URL: | https://joan-eject-command.tutorialkit-demo-page.pages.dev |
Deploying tutorialkit-docs-page with
|
| Latest commit: |
75925a8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://57f5341d.tutorialkit-docs-page.pages.dev |
| Branch Preview URL: | https://joan-eject-command.tutorialkit-docs-page.pages.dev |
|
|
||
| for (const dep of REQUIRED_DEPENDENCIES) { | ||
| if (!(dep in pkgJson.dependencies) && !(dep in pkgJson.devDependencies)) { | ||
| pkgJson.dependencies[dep] = astroIntegrationPkgJson.dependencies[dep]; |
There was a problem hiding this comment.
Maybe not super important for now, but we might want to check the version of the dependency and warn the user that they might need to upgrade/downgrade or something.
There was a problem hiding this comment.
We do! The let hasChanged = false; is used for that purpose.
There was a problem hiding this comment.
Hm so technically this can result in false positives if say I have nanostores in my package.json already but the range just doesn't match but the version would be the same, e.g.
^1.1.1 !== 1.1.1
Given that there's no new patch version and 1.1.1 is the latest. Wouldn't it be better to check node_modules or check the lock file? But maybe this is good enough and a warning is good in any case which would force the user to double check the dependency.
There was a problem hiding this comment.
If they have an incompatible version of nanostores, there's not much we can do as they'll have to update the code they've just ejected. Hopefully there's a TypeScript error so that they know.
If it's compatible, then this code uses their version, so I think this code is good?
There was a problem hiding this comment.
Yea I think it's fine cause we only show the warning if that dependency is missing entirely in the package.json. I was just wondering if we can be smarter here and maybe check the version and print a hint if the version doesn't match to tell them that there may be incompatibilities.
There was a problem hiding this comment.
Ah good point! Oh yeah that would be really nice actually.
There was a problem hiding this comment.
One possible way would be to leverage npm why --json to see what version is installed and what version we require.
| if (hasChanged) { | ||
| fs.writeFileSync(pkgJsonPath, JSON.stringify(pkgJson, undefined, 2), { encoding: 'utf-8' }); | ||
|
|
||
| console.log(primaryLabel('INFO'), 'New dependencies added, install the new dependencies before proceeding'); |
There was a problem hiding this comment.
Technically we could do this for them. Infer the package manager based on the lock file or something, or ignore if there are no node_modules yet? But I'm fine with this info for now.
There was a problem hiding this comment.
We could indeed to it for them but I kept it simple for now.
If there are no node_modules then the eject command won't work because it moves the files of the version of @tutorialkit/astro that they use (instead of using a possibily incompatible one).
|
|
||
| for (const dep of REQUIRED_DEPENDENCIES) { | ||
| if (!(dep in pkgJson.dependencies) && !(dep in pkgJson.devDependencies)) { | ||
| pkgJson.dependencies[dep] = astroIntegrationPkgJson.dependencies[dep]; |
There was a problem hiding this comment.
Hm so technically this can result in false positives if say I have nanostores in my package.json already but the range just doesn't match but the version would be the same, e.g.
^1.1.1 !== 1.1.1
Given that there's no new patch version and 1.1.1 is the latest. Wouldn't it be better to check node_modules or check the lock file? But maybe this is good enough and a warning is good in any case which would force the user to double check the dependency.
Co-authored-by: Dominic Elm <[email protected]>
Co-authored-by: Dominic Elm <[email protected]>
This PR adds an
ejectcommand to thetutorialkitCLI so that you can get back full control if you ever need to.It works by modifying the
astro.config.tsfile to turn off thedefaultRoutesand then copy the content of@tutorialkit/astro/dist/defaultinto thesrcfolder of the TutorialKit project.I renamed
env.d.tsinastro/src/default/toenv-default.d.tsso that they are no collision as they have different content. Ideally we should merge them but having two separate file is simpler to start with.Note that
--forcewill be required because of theenv.d.tsfor TutorialKit project that have already be generated. The collision will only be avoided for future projects.