Migrate codes to typescript and yarn to pnpm#45
Conversation
|
I can migrate them to typescript.
updateI could remove all |
856541c to
cbe59cc
Compare
29bdfb1 to
b7deca1
Compare
Temzasse
left a comment
There was a problem hiding this comment.
Thanks for the improvement suggestions! 👍🏻
I think it would be easier to review and merge these changes if we split them into two different PRs: one with the config changes (SWC/Jest stuff) and another with the TS changes in the internals.
I added some initial comments about the config changes but didn't yet have the time to look at the TS changes.
| 'react-native-svg', | ||
| ].join('|'); | ||
|
|
||
| export default { |
There was a problem hiding this comment.
I'm a little hesitant about switching to SWC since the current babel based setup works quite well and the time it takes to execute all the tests is only like 5 seconds so it doesn't take that long any way 🤔
There was a problem hiding this comment.
@Temzasse
Of course, I get lit of it, if you don't like it.
But, let me explain it to you a little, the configuration is very simple and easy to replace. All you need to do is @swc/jest to babel-jest if wanting to switch it, and it marks 5 times faster than babel-jest in my project's test which takes more than 3 minutes if using babel-jest or ts-jest. In my local environment of this project, it passed test less than 2 sec (1.77 s, estimated 2 s), (0.863 s, estimated 1 s with cache).
The case that SWC is fragile is when you compress and mangle codes (these features aren't needed if only you want to test) and with using decorator's metadata, and vercel is already using it in their library, Next.js. so in almost case you don't need to worry about it if you don't use conifgurations described above.
But, you don't feel pain to follow up this tool, I'll remove it.
| @@ -0,0 +1,31 @@ | |||
| const esmModules = [ | |||
There was a problem hiding this comment.
Why is this needed? We don't use any of these libs in the internals of the library.
There was a problem hiding this comment.
This is recommended configuration introduced by expo.
https://docs.expo.dev/guides/testing-with-jest/#configuration
If we use pnpm, the module resolution system is a bit different from npm and it caches the modules in node_modules/.pnpm, so need manually setting at least, '@react-native/' namespace. See react-native preset.
It's not high possibility to use another native-library, but it's ready for testing with third party libraries. If you don't need them, I remove them except for first react-native configuration (first array element configuration is
in high possibility to be required for test-libraries though react-native's jest preset narrower than it) which needs transpiled.
| "react-native": "0.69.6", | ||
| "react-native-builder-bob": "0.18.1", | ||
| "react-test-renderer": "18.0.0", | ||
| "ts-jest": "^29.0.3", |
There was a problem hiding this comment.
Are ts-jest and ts-node needed since .tsx files are transpiled by @swc/jest?
There was a problem hiding this comment.
No, they aren't.
The ts-jest is used for jest.config.ts. If you install it, you can write jest config files in typescript like jest-setup.ts, jest-environement.ts so on, and specify ts files in the jest.config file. ts-node is set peerDependency in ts-jest.@swc/jest itself can run without ts-jest and ts-node.
| "commonjs", | ||
| "module" | ||
| "module", | ||
| "typescript" |
There was a problem hiding this comment.
I think we don't want to let builder bob generate the type defs since we want to have the types be defined separately since it's practically impossible to get the type defs work correctly by inferring them from the internals.
There was a problem hiding this comment.
Oh, sorry this is my mistake.
I met test error at first after setting minimum configuration, and I thought this configuration needed. But, Only I needed was to rename index.ts to index.tsx to fix it. I'll remove it.
fix: remove yarn
b7deca1 to
6e132b5
Compare
|
Now, this PR includes SWC configuraiton and only rename js files to ts files with |
|
Furthermore I notice we need to modify github actions.
And https://github.com/Temzasse/stitches-native/actions/runs/3812284842/jobs/6485411228 shows it works fine. |
|
|
||
| - uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: '15' |
There was a problem hiding this comment.
This is changed to fix even number versioning for LTS, because odd number versioning of node is experimental and generally for development.
https://nodesource.com/blog/understanding-how-node-js-release-lines-work/
This repository almost doesn't depend on native library, so you can treat it as simple node_modules.
So, I try DX improved migrate some tools.
@swc/jestinstead of 'babel-jest' from react-native/jest-preset.js. The former is very faster than the latter.Please check benchmark. (e.g.: es5 so on)And there are additional points worth noting.
I don't still replace ambient files in types directories and it's used as primary exported types although I can do so. That means codes in internal emits js codes and sourcemap without type definitions as before but you can use typescript under development. If the current definitions depend on and need to followThis is done though loosely typing. But, the logic is almost same first JS file and if these types are wrong, the types are not used in the users' environment.@stitiches/reacttypes, we migrate writing them manually to emitting them from tsc, but it's enough now to define and use type advantage in internal. So I want to do that after you will have finished working athttps://github.com/Temzasse/stitches-native/tree/rewrite-media-stylesbecause you may feel convenient to use typescript and test improvements in the branch with merging this PR soon.