chore: Update build script to generate flow and TS types:
#Fixes #25
Using StyleX as a starting point, overhauling the build scripts.
- [x] Copy the
gen-flowwrapper aroundflow-api-translator - [x] Set up script to generate per-file Flow and TS type-defs
- [x] Update the
package.jsonto point to the new output files - [x] Delete the existing build steps
Skipped these steps as we decided to keep flat bundles for now:
- [ ] ~~Set up a Babel translation step for publishing to NPM that doesn't pre-bundle~~
- [ ] ~~Set up a Babel translation step that translates to Haste modules~~
compressed-size: runtime library
Size change: +15.90 kB 🔴 Total size: 15.90 kB
| Filename: gzip (minify) | kB size | kB change | % change | |
|---|---|---|---|---|
./packages/react-strict-dom/dist/dom/index.js |
2.90 (8.66) | +2.90 (+8.66) | +100.0% (+100.0%) | 🔴 |
./packages/react-strict-dom/dist/dom/runtime.js |
0.92 (2.26) | +0.92 (+2.26) | +100.0% (+100.0%) | 🔴 |
./packages/react-strict-dom/dist/native/index.js |
12.08 (37.79) | +12.08 (+37.79) | +100.0% (+100.0%) | 🔴 |
Nice! I kind of want to keep flat bundling for OSS. And I wish this wasn't so much more complicated than the current setup (apart from existing flow lib def script). This isn't a modular library and we've previously found benefits in size and app bundling perf from pre-bundling libraries like react. Plus, to retain the most useful perf CI data we need flat bundles. AFAICT the motivation for this change is because flow doesn't support automatically generating lib defs
I kind of want to keep flat bundling for OSS
One downside of flat bundling is that you can take advantage of tree-shaking in modern bundlers.
However, if we stop here, we can keep the flat bundles for the JS code and only keep the separate files for the type-defs.
This isn't a modular library
If we only publish an ESM build to NPM, we can limit what we export so that all the other files are "private".
AFAICT the motivation for this change is because flow doesn't support automatically generating lib defs
The flow-api-translater package being used by the script is the Flow teams solution for generating lib-defs. The other motivation is that this lets us generate TS type-defs as well. A folder of file like this is fully supported in both Flow and TS and so we don't really gain anything by bundling them into flat files.
There's nothing to tree-shake out of this package on web, or even native really. AFAIK, tree-shaking / DCE isn't limited to what's in different files and should work if exports within a file aren't used. So we can focus on the types side of things
@necolas That makes things simple enough. This PR is now ready.
I simply changed the output path from dist/dom.js to dist/dom/index.js and not the separate type files work as expected. The dist/dom/index.d.ts and dist/dom/index.js.flow files shadow the actual file and that works for both TS and Flow.
I can probably make this set up work for internal sync as well.
Update: Made a couple of changes to ensure that no two files in the package have the same name. (Except when there are dom and native counter-parts`). This was mostly true already except:
- There was one unused file
- There are two versions of
flattenStyle.jsthat work similarly but with different types.
For now, I've renamed one of the flattenStyle.js to flattenStyleXStyles.js, but eventually, it should be deleted.
This work has been done to be compatible with Haste for internal sync.
Not able to reproduce this failure locally. I don't even have the file that is erroring. :(
Not able to reproduce this failure locally. I don't even have the file that is erroring. :(
I can reproduce once this PR is rebased on main
@necolas Does this look good to merge? (I know you're on a break, so let me know if I should run this by someone else while you're away)
There's still a few things to update before that. I'll comment over the weekend. Also left some questions above
Does this patch still allow importing of types from the main package, e.g., import type { StrictHTMLElement } from 'react-strict-dom'?
Anything that is exported from the entry point can be imported directly. If there are types that are currently not exported from the entrypoint that are supposed to be public, we can add them.
The paths to the build files also need to be updated in the performance workflow file.
^ this still needs to be done.
Also need to update the paths in the build size badges in the RSD README. They will update to the correct values once I publish the release after this patch
I pushed fixes for the path updates.
However, there's one more path in the .flowconfig - https://github.com/facebook/react-strict-dom/blob/main/.flowconfig#L15. Updating that to point to the new flow entry point results in flow errors which should show up in CI for that commit. Removing the mapping (or mapping to a non-existent file, as was the case before in this PR) results in no flow errors...which is a bit concerning.
Let me figure this out.
Removing the mapping (or mapping to a non-existent file, as was the case before in this PR) results in no flow errors...which is a bit concerning.
This is a known issue with Flow. It causes "untyped" code rather than type errors. We might need to add some checks to cache type-coverage regressions for this. We also need to update the Flow version. I'll do that after this PR has merged.
Thanks! Merged