feat(typescript)!: support module augmentation of the exposed types - #1147
Conversation
Add a ts-support check that augments CellStyle with a custom style property and uses it against the Stylesheet API. The check guards the extension point in both directions: it fails if the augmented types go back to being type aliases, and a @ts-expect-error on an undeclared property makes sure the augmentation is what lets the rest compile, rather than a type that would accept anything. Raise the TypeScript version of the ts-support package from 3.8.2 to 3.9.10, and the version documented in the README and on the website accordingly. Module augmentation is broken on 3.8: the augmented types reach the entry point through an "export * " re-export, and 3.8 overrides the declaration instead of merging with it, silently leaving the augmented type with the added property only. Nothing is reported where the augmentation is declared, the failures surface later as unrelated assignability errors. 3.9 merges as expected. BREAKING CHANGE: - The minimum supported TypeScript version is now 3.9, up from 3.8. Applications still on TypeScript 3.8 must upgrade to use this release.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 4 reviews per rolling hour; 2 remain after this review. WalkthroughThe PR raises the current minimum TypeScript version to 3.9, updates the TypeScript support package, adds a module augmentation type test, and documents TypeScript 3.8 support for version 0.24.0 and earlier. ChangesTypeScript compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change raises the minimum supported TypeScript version to 3.9 to enable reliable module augmentation and documents the breaking requirement; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ts-support/src/module-augmentation.ts (1)
17-19: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd coverage for the
CellStateStyleAPI contract.In
packages/core/src/view/style/Stylesheet.ts, Lines 152-154,putCellStyleacceptsCellStateStyle. The current test passes a variable already typed asCellStyle, so structural assignability makes the call compile without proving that the custom property is accepted on aCellStateStyleobject literal.Add a
CellStateStyleaugmentation test that passes an object literal toputCellStyle. Keep the directCellStyletest as coverage for the exposed interface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a7c3bbb-ee10-4a92-9d0d-5de4c8082c52
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
CHANGELOG.mdREADME.mdpackages/ts-support/package.jsonpackages/ts-support/src/module-augmentation.tspackages/website/docs/getting-started.mdxpackages/website/docs/intro.md
Included review availability: Your plan includes up to 4 reviews per rolling hour; 2 remain after this review.
CellStyle extends CellStateStyle, so augmenting the base interface makes the added property available on both, and on everything else built on CellStateStyle (CellState.style, the Stylesheet default and named styles). Augmenting CellStyle only proved that the derived type could be extended, which covers fewer of the ways users actually declare custom style properties.
Readers of the breaking change have no way to judge how disruptive it is without knowing how old TypeScript 3.8 and 3.9 are. State the release dates so they can see that both lines are more than six years old and that few applications should still be pinned to 3.8.
Drop the patch versions, since the documented minimum is the 3.9 line and not a specific patch, so naming 3.8.2 and 3.9.3 only added noise. Give the dates of the first GA of each line instead, and add TypeScript 4.0 to show that 3.9 was superseded three months after its release, which is what actually tells readers how few applications the bump affects.
|
Module augmentation of the maxGraph types silently does not work on TypeScript 3.8, where the augmentation overrides the declaration instead of merging with it. TypeScript 3.9 fixes it, so the minimum supported version moves from 3.8 to 3.9, in
ts-supportas well as in the README and on the website.TypeScript 3.8 was released in February 2020, 3.9 in May 2020 and 4.0 in August 2020. Both 3.8 and 3.9 are more than six years old, and 3.9 was superseded three months after its release, so most applications already use a newer version and the impact of this breaking change should be limited.
Adds a
ts-supportcheck that augmentsCellStateStylewith a custom style property and passes it to theStylesheetAPI through aCellStyle.CellStyleextendsCellStateStyle, so augmenting the base interface makes the property available on both, and on everything else built onCellStateStyle(CellState.style, theStylesheetdefault and named styles). The check fails if these types go back to being aliases, and a@ts-expect-erroron an undeclared property makes sure the augmentation is what lets the rest compile.BREAKING CHANGE: applications on TypeScript 3.8 must upgrade to 3.9.
Summary by CodeRabbit
Documentation
Tests