chore: migrate to Plop v3 #573
Conversation
Co-authored-by: Louis Bompart <[email protected]>
Co-authored-by: jpmarceau <[email protected]>
Co-authored-by: Yassine <[email protected]>
This reverts commit b50e42d.
ThibodeauJF
requested review from
louis-bompart,
olamothe and
y-lakhdar
as code owners
|
Thanks for your contribution @ThibodeauJF !
|
| @@ -1,6 +1,6 @@ | |||
| import {NodePlopAPI} from 'plop'; | |||
| import {spawn} from 'child_process'; | |||
| import {getPackageManager} from './utils'; | |||
| import {getPackageManager} from './utils.js'; | |||
There was a problem hiding this comment.
Why not translate utils to TypeScript?
There was a problem hiding this comment.
it is in TS, but we need to set it properly to the output with a .js in order for it to work with ES2020, like the links indicate. I'll try with nodenext, though
|
Pull Request Report PR Title ✅ Title follows the conventional commit spec. |
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "module": "commonjs", | |||
| "module": "ES2020", | |||
There was a problem hiding this comment.
Can you try with nodenext ? would require typescript 4.5 but it should be the best way now I reckon.
There was a problem hiding this comment.
You mean 4.6? Doesn't appear to work with 4.5. What's the issue with ES2020 ?
| @@ -1,6 +1,6 @@ | |||
| import {NodePlopAPI} from 'plop'; | |||
| import {spawn} from 'child_process'; | |||
| import {getPackageManager} from './utils'; | |||
| import {getPackageManager} from './utils.js'; | |||
There was a problem hiding this comment.
Why not translate utils to TypeScript?
| @@ -0,0 +1,29 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Why do you need the shebang? 🤔
There was a problem hiding this comment.
just copied from their docs too
There was a problem hiding this comment.
I'm curious abt it, because shebang does not work on windows soo...
There was a problem hiding this comment.
Seems like a good thing to keep https://stackoverflow.com/a/33510581
This way, maybe when i install npm i -g @coveo/create-atomic and then run create-atomic myproject it will invoke node correctly 🤷 I haven't tested, just thinking out loud
That's a harsh requirement, I wasn't on it until recently |
True, it is more stringent, however, because NodeJS and ECMA as a whole is turning their back on CJS and moving toward ESM modules, I think that I would prefer to limit support on Node 16 on the command, and not have to rework it next year Overall, my two cents:
This position is motivated by the rising number of packages that we cannot use because we're stuck with CJS module and engine. I prefer for us to be a step in advance than having to do the work twice. @olamothe , @y-lakhdar, what do you think? |
https://coveord.atlassian.net/browse/CDX-716