feat!: migrate @doist/react-interpolate to tsdown#56
Conversation
2842d3d to
30d2256
Compare
doistbot
left a comment
There was a problem hiding this comment.
This pull request successfully migrates the build pipeline from the legacy Rollup and Babel setup to tsdown while maintaining dual ESM/CJS outputs and updating the associated configurations. This transition cleans up legacy dependencies and modernizes the build tooling, which is a great step forward for the repository's long-term maintainability. However, there are a few areas to refine, including ensuring the test matrix aligns with the declared Node engines, restoring the browser compatibility target to prevent breaking changes with ES2020 syntax, syncing the CI Node version with .nvmrc, and strengthening the package compatibility tests to validate the actual installed tarball.
f80f8ea to
77c1045
Compare
doistbot
left a comment
There was a problem hiding this comment.
This PR modernizes the package build pipeline with tsdown and updates the compatibility contract for newer Node environments. These updates significantly streamline the build process and align the published artifacts with modern toolchains. There are just a few remaining adjustments needed to remove an outdated self-referencing smoke test and ensure CI fully covers the newly added Node 24 compatibility range.
| async function main() { | ||
| const cjsPackage = require('..') | ||
| const esmPackage = await import('../dist/react-interpolate.mjs') | ||
| const cjsPackage = require('@doist/react-interpolate') |
There was a problem hiding this comment.
[P2] This file still relies on Node self-referencing instead of testing a packed tarball. Since you noted in the previous review that this smoke test would be removed, it appears it was mistakenly modified instead. If it's no longer needed, remember to delete this file and remove its execution from the test:package script.
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js ${{ matrix.node-version }} | ||
| - name: Setup Node.js from .node-version |
There was a problem hiding this comment.
[P2] This workflow now only exercises the single version pinned in .node-version, but package.json newly declares support for >=24.0.0 as well. That means the Node 24 part of the published engines contract is untested in CI. Add a second 24.x job/matrix entry here, or narrow engines.node to the version(s) you actually verify.
Summary
tsdown, keep dual ESM/CJS output, and publish an explicitexportscontract for the package.src/, and keep package-compat coverage focused on the package entrypoint shape.^22.18.0 || >=24.0.0, switch CI/release workflows to a single.node-versionsource of truth, and stop preserving the previous IE11-era transpilation baseline.Breaking Changes
^22.18.0 || >=24.0.0.Validation
npm run typechecknpm test -- --runInBandnpm run lintnpm run buildnpm run test:packagenpx publintnpm packrequire('@doist/react-interpolate')andawait import('@doist/react-interpolate')Reference