fix(crwa): decouple JS preference from yarn install step, remove yarn1 option
#8191
Conversation
|
|
||
| // Engine check | ||
| await executeCompatibilityCheck(templateDir, yarnInstall) | ||
| await executeCompatibilityCheck(path.join(templatesDir, 'ts'), yarnInstall) |
There was a problem hiding this comment.
The engines will never be different between the TS and JS templates, but since the TS one is the source of truth, we use it here for the engines check.
| @@ -27,7 +27,7 @@ | |||
| }, | |||
| "typeRoots": ["../node_modules/@types", "./node_modules/@types"], | |||
| "types": ["jest", "@testing-library/jest-dom"], | |||
| "jsx": "preserve", | |||
| "jsx": "preserve" | |||
There was a problem hiding this comment.
This trailing comma actually isn't valid JSON. VS Code doesn't complain but Node.js APIs do.
There was a problem hiding this comment.
Despite the possible overhead (keeping the JS and TS templates in sync; adding code to the package), I think this is the most elegant and performant solution. Well done 👍
A couple of change requests in the comments for you to look at.
Lastly, could you add a GH Action similar to the "update fixture" that looks for changes in /templates and requires a manual label to "approve". That plus the release tooling step you're adding is enough sanity check for me.
🚀
|
Did we consider a standalone |
|
Haven’t explored that solution yet. Just wanted to see what this entailed. Something I didn’t like about a dlx or npx command is that it’s a dlx or npx command—another install step. And here you’re not paying the price of dependencies either, unless you mean the extra template files which I don’t think adds up to much but I’ll confirm the exact size increase. And we would need framework-side changes for ts-to-js as a dlx or npx command: we’d be publishing another package. (Not that that’s difficult or anything) |
|
Another idea: Can the templates be separate packages? Added benefit is that you can then do |
| @@ -14,7 +14,7 @@ | |||
| "check": "yarn node ./tasks/check/check.mjs", | |||
| "clean:prisma": "rimraf node_modules/.prisma/client && node node_modules/@prisma/client/scripts/postinstall.js", | |||
| "e2e": "node ./tasks/run-e2e", | |||
| "lint": "RWJS_CWD=packages/create-redwood-app/template eslint --config .eslintrc.js packages", | |||
| "lint": "RWJS_CWD=packages/create-redwood-app/templates/ts eslint --config .eslintrc.js --ignore-pattern Routes.js packages", | |||
There was a problem hiding this comment.
yarn lint complains about the Routes.js file in the JS template. It says something like "NotFoundPage" isn't defined, which is true, but it doesn't understand that Redwood projects take care of importing it. While we need a more straightforward linting setup, I don't think it should be done in this PR
…arn1` option (#8191) * wip * fix: decouple js preference from yarn install * add tests * rm ts2js export * fix lint and lint fix * fix run e2e script * test project script updates * add back ts-to-js * add .nvmrc back * deprecate ts to js * add back ts2js in internal * use babel instead of esbuild to preserve comments * lint fix * add prettier step, .nvmrc file to js template * style * fix yarn check * update crwa snapshot * ignore js template routes file in linting * fix yarn versions * fix tsconfig to jsconfig transformation * handle script files * wip update release command * log section * comment delete jsConfig allowJS line * wip * add action to catch changes to crwa ts template * style
In
yarn create redwood-app, the JS language preference has a dependency on the install step (which isn't without its issues #8164). This means if install fails, the user gets a TS project. This is because we convert the TS template to JS on the fly, using the Redwood CLI'sts-to-jscommand, so the CLI has to be installed for us to run int. This coupling isn't ideal.We thought about a few ways of decoupling the JS language preference from the install step. Here I'm proposing we add a JS template to the
create-redwood-apppackage. This JS template is derived from the TS template, so that one remains the source of truth. Basically, I ran thets-to-jscommand (now a script increate-redwood-app) and am just caching its result.The tradeoff here is
create-redwood-appI think both of these tradeoffs are worth it.
This PR also
yarn create redwood-appts-to-jscommand from the CLI.nvmrcfile in the templateThis reason this PR has a lot of files changed is that I've nested the existing template inside a new
templates/tsdirectory. The diff also isn't too helpful since git seems a little confused about which files were moved vs which ones are new, so I've left comments where I thought they'd be helpful.Todos: