chore: release 3.0.0#61
Conversation
doistbot
left a comment
There was a problem hiding this comment.
This PR prepares the 3.0.0 release for @doist/react-interpolate by promoting recent changes, adding React 19 support, and introducing separate ESM and CJS builds. These updates do a great job modernizing the package's compatibility and distribution format. There are a few areas noted for refinement before publishing, specifically around relaxing the restrictive Node engine requirements, adjusting the MappingRenderFunction and SyntaxRule types to better enforce their runtime contracts, and expanding test coverage to fully validate the new ESM bundle, React 19 compatibility, and lexer error paths.
Some low priority findings & nits have been included in the summary below
Lower-priority review note (1)
- [P3] src/interpolate.test.tsx:9: Explicitly calling
cleanup()here is unnecessary. Because Vitest is configured withglobals: true,@testing-library/reactautomatically registers and runscleanup()in the globalafterEachhook.
|
|
||
| export interface SyntaxRule<T extends SyntaxTokenType = SyntaxTokenType> { | ||
| type: T; | ||
| regex: RegExp; |
There was a problem hiding this comment.
[P2] SyntaxRule exposes a raw RegExp, but lexer relies on two hidden invariants this type doesn't capture: the regex must be global and must put the token name in capture group 1. That means custom syntax can type-check and still fail immediately at runtime. Consider keeping this shape internal, or exposing helper constructors/validated builders so the public API enforces the same contract as the implementation.
| ]); | ||
| }).toThrow("Syntax rule regex must use the global flag"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
[P2] The new lexer.ts implementation throws "Syntax rule must capture a token name" if a custom syntax regex lacks a capture group. This new error path is currently untested. Consider adding a test case to this describe block to verify the behavior.
|
|
||
| assert(typeof cjsPackage === "object", "CJS package should be a namespace object"); | ||
| assert(typeof cjsPackage.default === "function", "CJS default export should be a function"); | ||
| assert(typeof esmPackage.default === "function", "ESM default export should be a function"); |
There was a problem hiding this comment.
[P2] The smoke check imports the ESM bundle but never executes it. With this PR generating separate ESM and CJS builds, an ESM-only runtime regression would still pass here as long as the symbol exists. Render esmPackage.default once too so both published entry points get real runtime coverage.
2f3e500 to
a606da0
Compare
Promotes the current next branch changes to main and bumps
@doist/react-interpolateto 3.0.0 for the manual release workflow.