“Vendor” what we need from tidy
#1186
Assignees
Labels
DX
Developer experience
Comments
dhess
added a commit
that referenced
this issue
May 4, 2024
In this commit, we remove the `@zxch3n/tidy` dependency in favor of our own hard fork `@hackworthltd/tidyt-wasm` and a bit of vendoring for the non-Wasm bits. Specifically: 1. Drop `@zxch3n/tidy`. 2. Add a `@hackworthltd/tidyt-wasm` dependency, which now provides the Wasm bits that `@zxch3n/tidy` previously did. 3. Vendor `tidy.ts` from `@zxch3n/tidy`, as this is the only bit of upstream that we need. (We also use `dispose.ts`, but that itself was vendored in `@zxch3n/tidy`, and we expect it's temporary, anyway; see #1187.) 4. Use a Vite plugin for loading Wasm modules, so we can drop the boilerplate `initWasm` step that `@zxch3n/tidy` requires. Note that we vendor `tidy.ts` from this upstream commit: zxch3n/tidy@54382fa Our vendored `tidy.ts` is nearly identical to upstream at that commit, except that a) we don't need the `initWasm` stuff, as explained above, and b) we pull the `visit` function into the file as well. (The latter lives in `utils.ts` in the upstream project.) Note that there are a few TypeScript errors in our vendored `tidy.ts`. These errors are also present in the upstream version, but they do not manifest in the running code, which appears to work identically to the upstream code it replaces. We will fix these TypeScript errors in a subsequent commit, but not here, as I want the diff vs. upstream to be as clean as possible in this initial vendoring commit. Fixes #1186. Signed-off-by: Drew Hess <[email protected]>
|
Closed by #1190 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Most of what we want out of our upstream
tidydependency is the Rust (Wasm) layout engine, plus a few of the TypeScript wrappers around it; and even the TypeScript bits are probably unnecessary if we just bite the bullet and write our own, as we currently have to create a Tidy layout and then tweak it for our unique “Primer” layout (e.g., for “right children”). This is surely inefficient.Given that the upstream
tidypackage is a combination of the Rust bits, the handful of TypeScript types and functions that it exports, and then a bunch of extra code in the form of a Storybook and a test app; and given that upstream hasn’t published it to a package repo, and it’s therefore a bit difficult to use as a dependency, I think we should do the following:npmpackage (we need not publish this to Npm.js; GitHub’s org-levelnpmpackage repo will suffice); andprimer-app.Once this refactoring is complete, we would only have a dependency on the Rust/Wasm code, which, as our new https://github.com/hackworthltd/nix-rust-wasm-npm repo demonstrates, is easy to hack, build, test, benchmark, and publish using our familiar Nix tools. There would be no need for us to maintain any of the extraneous bits of the current upstream package, and we would only have a single, straightforward dependency on our new Rust/Wasm-specific
npmpackage. Vendoring the leftovers would put us on course to make a more efficient version of the TypeScript bits, tailored for our particular needs.The risk is that if upstream adds major new features to
tidy, those might be difficult to sync up. However, this scenario currently seems unlikely, and as I explained above, I think it’s much more likely that we will want to do our own tweaks/rewrites which are too implementation-specific to be useful to upstream.Note that if we implement this plan, we will need to license the vendored bits of the upstream package in our code base separately from our own AGPL’ed code, but this will only require a small note in the project
READMEand some boilerplate license comments in the vendored code in our tree.The text was updated successfully, but these errors were encountered: