Upgrade to tsdx 2 so builds emit no invalid pure annotation - #105
Open
lukaselmer wants to merge 2 commits into
Open
Upgrade to tsdx 2 so builds emit no invalid pure annotation#105lukaselmer wants to merge 2 commits into
lukaselmer wants to merge 2 commits into
Conversation
Bundlers that follow pure annotations strictly, such as Rolldown (Vite 8), warn on every build: [INVALID_ANNOTATION] A comment "/*#__PURE__*/" in "react-async-hook/dist/react-async-hook.esm.js" contains an annotation that Rolldown cannot interpret due to the position of the comment. The two offending lines are _iteratorSymbol and _asyncIteratorSymbol from babel-plugin-transform-async-to-promises. tsdx 0.x ran that plugin to downlevel async to ES5, and a try block inside an async function made it pull in its helper module, which carries those two declarations. The annotation sits on a conditional expression, where it means nothing, so bundlers both reject it and keep the dead code it was meant to let them drop. tsdx 2 replaces the rollup and babel pipeline with bunchee, and the tsconfig target moves to ES2022, so async, await and try/finally survive to the output untouched. No downleveling, no helper module, no annotation. The hooks keep their exact source: this is a toolchain change only. The target matters as much as the tooling. bunchee reads the tsconfig, so at the old es5 target it still downleveled, only with SWC helpers rather than babel ones. That alone removes the warning, since the SWC helpers carry no misplaced annotation, but it keeps a needless helper chunk next to the bundle. ES2022 drops both. - tsdx 0.7.2 to 2.0.0. It shells out to bunchee, vitest and oxlint through bunx without depending on them, so bunchee and vitest are declared here; without them the build and the test run fail with ENOENT - typescript 3.4.5 to 6.0.2, the newest release that still ships the JavaScript compiler API bunchee needs to emit declarations. TypeScript 7 is the native port and drops that API, so it additionally requires the @typescript/typescript6 compatibility package; staying on 6 keeps a single TypeScript dependency - tsconfig target ES2022 and moduleResolution bundler, matching what tsdx 2 documents. importHelpers goes with tslib, which nothing needs once there is no downleveling - tests move from jest to vitest, which is what tsdx 2 runs: jest.fn becomes vi.fn, and vitest.config.mts supplies the globals and the jsdom environment that tsdx 0.x passed as --env=jsdom. The config is .mts because this package is not type: module - @types/jest and tslib go, both left without a consumer - node engine to >=20, as required by tsdx 2 Output filenames are unchanged, because bunchee reads main and module. Adds a types field next to typings, which bunchee needs to emit declarations. Adds the first tests for useAsyncAbortable, covering the signal, the abort on params change, a synchronous throw and a non-promise return. Verified with Rolldown: 2 INVALID_ANNOTATION warnings before, no warnings at all after. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Author
|
I've already noticed the issues in the build pipeline 9:56:40 PM: [1/5] Validating package.json... I'm on it |
There was a problem hiding this comment.
Pull request overview
This PR upgrades the library’s build/test toolchain (notably moving to tsdx@2) to address Rollup warnings about invalid /*#__PURE__*/ annotations in the published build output, while also expanding test coverage for useAsyncAbortable.
Changes:
- Add Vitest configuration and migrate tests to Vitest-style mocks (
vi) while adding newuseAsyncAbortabletest cases. - Update TypeScript compiler settings (ES2022 target +
bundlerresolution) to align with the newer build toolchain. - Update package tooling/dependencies (Node engine >= 20, TSDX 2, TypeScript 6, Vitest, JSDOM).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
vitest.config.mts |
Introduces Vitest configuration (globals + jsdom). |
tsconfig.json |
Updates TS target/module settings and module resolution mode for the new toolchain. |
test/useAsync.test.ts |
Switches mocking from jest.fn() to vi.fn() and adds useAsyncAbortable tests. |
package.json |
Upgrades toolchain dependencies (tsdx/typescript/vitest) and updates Node engine + test script. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
164
to
167
| // Test added because Jest mocks can return promises that arre not instances of Promises | ||
| // This test ensures better testability of user code | ||
| // See https://github.com/slorber/react-async-hook/issues/24 | ||
| it('should resolve a successful Jest mocked resolved value', async () => { |
Comment on lines
+82
to
+84
| "tsdx": "^2.0.0", | ||
| "typescript": "^6.0.2", | ||
| "vitest": "^4.1.10" |
Author
I don't think I have permissions to change it :) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To resolve #103, I've tested a few approches (using AI/Copilot).
I like this solution the best as it only updates the build tools, instead of changing the source code. The additional
useAsyncAbortabletests also make sense to me.To verify the changes, I've built the library locally, and ran our automated test suite with the built version for multiple projects I'm currently working on. I didn't test it manually in my local browser (let me know if I should do that, and I'll do it).
Caveat: it changes the target from es5 to ES2022. I think that's OK in 2026. It could be a good idea to bump the major version because of this.
I've also tried the following approaches to fix #103:
PS: awesome newsletter, I've been reading it for years 👍