Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of using declarations in our vendored tidy #1187

Closed
dhess opened this issue May 2, 2024 · 1 comment · Fixed by #1192
Closed

Make use of using declarations in our vendored tidy #1187

dhess opened this issue May 2, 2024 · 1 comment · Fixed by #1192

Comments

@dhess
Copy link
Member

dhess commented May 2, 2024

Upstream tidy uses the TypeScript “disposable” pattern by copy-pasting some boilerplate-y code: https://github.com/zxch3n/tidy/blob/master/src/dispose.ts

If I understand correctly, this pattern is built into (in preview form) recent versions of TypeScript: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-2.html. It can be enabled as follows:

{
    "compilerOptions": {
        "target": "es2022",
        "lib": ["es2022", "esnext.disposable", "dom"]
    }
}

Assuming we implement #1186, we should look into enabling this and refactoring the vendored code to use it.

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]>
@dhess
Copy link
Member Author

dhess commented May 4, 2024

I don't think we can easily use using, but we can use the built-in Disposable interface.

dhess added a commit that referenced this issue May 4, 2024
dhess added a commit that referenced this issue May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant