feat: add jest recipe #4
Conversation
km1chno
force-pushed
the
feat-add-jest-recipe
branch
from
c86c3cc
to
336fdbb
Compare
km1chno
force-pushed
the
feat-add-jest-recipe
branch
from
336fdbb
to
92ded7b
Compare
src/commands/react-native-ci-cli.ts
Outdated
| import isGitDirty from 'is-git-dirty' | ||
|
|
||
| const command: GluegunCommand = { | ||
| name: 'react-native-ci-cli', | ||
| run: async (toolbox) => { | ||
| const { intro, confirm, outro } = await import('@clack/prompts') |
There was a problem hiding this comment.
So I don't really like it, but the story is the following. When trying to use p-map, it turned out that it is a ES module and therefore it has to be imported, not required. The problem is that with tsconfig we had, specifically with module: commonjs and moduleResolution: node, the transpiler automatically converted import syntax to require, resulting in an error.
According to https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext, the recommended way to handle such cases is to change module and moduleResolution to node16 and target to es2022 and then use static import for CommonJS dependencies and dynamic asynchronous import(...) for ESM.
We could just not use p-map, but the problem would eventually come back since a lot of packages are ESM only. Also, I still don't understand it that well, because clack is ESM and importing it worked just fine with the old tsconfig. I guess since module: commonjs is not recommended (according to the link above), maybe it is just not consistent.
Let me know what you think and whether you had such problems in the past and may know a better solution:)
There was a problem hiding this comment.
The current solution looks good to me - It's always a pain, so I'd suggest looking for modules that are not ESM
There was a problem hiding this comment.
I played with it for a bit and it might be tricky to set it up. Clack works probably because it is transpired to both .cjs and .mjs, I tried to import just pMap with dynamic import but it doesn't work probably due to microsoft/TypeScript#43329. Tbh, dynamic imports everywhere look massy, I would revert to using you helper instead of pMap, just if you could extract it to some util and maybe try to write using async/await. What do you think?
There was a problem hiding this comment.
After discussion with @maciekstosio, we decided to revert back to previous tsconfig and just not use p-map since it's easily avoidable. If in the future we will need a crucial ESM only dependency, then we will migrate to module: node16. I reverted the changes in 1e39c1e
src/commands/react-native-ci-cli.ts
Outdated
|
|
||
| const executorResults = await Promise.all( | ||
| executors.map((executor) => executor(toolbox)) | ||
| const executorResults = await executors.reduce( |
There was a problem hiding this comment.
One last think, let's put it in some utils (not necessarly extension as I think it's not strictly related tool for CLI)
There was a problem hiding this comment.
extracted to /src/utils/sequentialPromiseMap.ts in 7ddc6c5
package.jsonto extensions:extensions/dependencies.tsandextensions/scripts.ts,jestin PR.