Repository navigation
[quantity-formatting] Optional unitsProvider, startup race fix, missing peers, exported prop types - #1828
Open
hl662 wants to merge 11 commits into
Open
[quantity-formatting] Optional unitsProvider, startup race fix, missing peers, exported prop types#1828hl662 wants to merge 11 commits into
hl662 wants to merge 11 commits into
Conversation
…, exported prop types, error logging, stale keys - QuantityFormatting.startup shares one in-flight promise; localization getter throws a descriptive error before startup - Add @itwin/core-common and @itwin/ecschema-metadata peers (public types already depend on them) - Export component prop types and document onFormatChange timing - Log formatter/persistence-unit failures instead of swallowing them - FormatSetSelector keys no longer shift while searching; FormatSetPanel initializes from props - Persistence unit reload keyed on unit name; remove dead code and unused imports; enable noUnusedLocals
- Guard QuantityFormatting startup completions with a generation so terminate() invalidates in-flight work; document first-call-wins localization - Tests: deferred startup race, FormatSetSelector stable keys while filtering, persistence-unit reload/logging
…elApp.quantityFormatter.unitsProvider The components follow onUnitsProviderChanged when the prop is omitted, so test-viewer drops its two copies of that wiring.
hl662
added this pull request to stack #1829
October 8, 2026 03:01
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A startup completing after termination can leave its localization namespace registered indefinitely.
1 open finding
What changed in this PR
Improves quantity-formatting initialization, provider defaults, public types, diagnostics, and cleanup.
Changes:
- Makes unit providers optional and reactive to application-level changes.
- Prevents concurrent startup races and exports component prop types.
- Adds peer dependencies, diagnostics, tests, and unused-code checks.
| File | Description |
|---|---|
.changeset/@itwin-quantity-formatting-react-package-contract.md |
Records package contract changes. |
.changeset/@itwin-quantity-formatting-react-optional-units-provider.md |
Records optional provider support. |
apps/test-viewer/src/components/quantity-formatting/FormatPropertyContextMenuItem.tsx |
Uses the default units provider. |
apps/test-viewer/src/components/quantity-formatting/FormatTabPanel.tsx |
Removes explicit provider plumbing. |
apps/test-viewer/src/components/quantity-formatting/QuantityFormatButton.tsx |
Removes duplicated provider subscription. |
packages/quantity-formatting/README.md |
Documents optional providers. |
packages/quantity-formatting/api/quantity-formatting-react.api.md |
Updates extracted API declarations. |
packages/quantity-formatting/api/quantity-formatting-react.exports.csv |
Adds exported prop types. |
packages/quantity-formatting/package.json |
Adds missing peer dependencies. |
packages/quantity-formatting/src/QuantityFormatting.ts |
Coordinates startup and localization state. |
packages/quantity-formatting/src/components/quantityformat/FormatPanel.tsx |
Supports the default provider and exports props. |
packages/quantity-formatting/src/components/quantityformat/FormatSample.tsx |
Supports defaults and logs formatter failures. |
packages/quantity-formatting/src/components/quantityformat/FormatSelector.tsx |
Exports selector props. |
packages/quantity-formatting/src/components/quantityformat/FormatSetPanel.tsx |
Initializes supplied values and exports props. |
packages/quantity-formatting/src/components/quantityformat/FormatSetSelector.tsx |
Stabilizes fallback keys and exports props. |
packages/quantity-formatting/src/components/quantityformat/QuantityFormatPanel.tsx |
Resolves providers and optimizes unit loading. |
packages/quantity-formatting/src/components/quantityformat/internal/FractionDash.tsx |
Removes an unused handler. |
packages/quantity-formatting/src/components/quantityformat/internal/StationBaseFactor.tsx |
Removes an unnecessary type escape. |
packages/quantity-formatting/src/components/quantityformat/internal/misc/DecimalPrecision.tsx |
Removes an unused import. |
packages/quantity-formatting/src/components/quantityformat/internal/misc/DecimalSeparator.tsx |
Removes an unused import. |
packages/quantity-formatting/src/components/quantityformat/internal/misc/FractionPrecision.tsx |
Removes an unused import. |
packages/quantity-formatting/src/components/quantityformat/internal/misc/SignOption.tsx |
Removes an unused import. |
packages/quantity-formatting/src/components/quantityformat/internal/misc/StationSizeSelector.tsx |
Removes an unused import. |
packages/quantity-formatting/src/components/quantityformat/internal/useUnitsProvider.ts |
Adds provider resolution and subscription. |
packages/quantity-formatting/src/quantity-formatting-react.ts |
Exports public prop types. |
packages/quantity-formatting/src/test/QuantityFormatting.test.ts |
Tests startup lifecycle behavior. |
packages/quantity-formatting/src/test/_Setup.test.ts |
Removes an unused test import. |
packages/quantity-formatting/src/test/quantityformat/FormatSample.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/FormatSetSelector.test.tsx |
Tests stable filtered selection. |
packages/quantity-formatting/src/test/quantityformat/QuantityFormatPanel.test.tsx |
Tests reset, loading, and logging behavior. |
packages/quantity-formatting/src/test/quantityformat/internal/DecimalSeparator.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/FormatPrecision.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/FractionDash.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/KeepDecimalPoint.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/KeepSingleZero.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/ShowTrailingZeros.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/SignOption.test.tsx |
Removes unused test imports. |
packages/quantity-formatting/src/test/quantityformat/internal/StationBaseFactor.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/ThousandsSeparator.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/ZeroEmpty.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/internal/useUnitsProvider.test.ts |
Tests provider resolution and updates. |
packages/quantity-formatting/src/test/quantityformat/panels/AzimuthPanel.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/panels/BearingPanel.test.tsx |
Removes unused test imports. |
packages/quantity-formatting/src/test/quantityformat/panels/DecimalPanel.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/panels/FractionalPanel.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/panels/RatioPanel.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/panels/ScientificPanel.test.tsx |
Removes an unused import. |
packages/quantity-formatting/src/test/quantityformat/panels/StationPanel.test.tsx |
Removes an unused import. |
packages/quantity-formatting/tsconfig.json |
Enables unused-code checks. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
…ration that completes after terminate Keeps the registration when a newer startup reuses the same localization instance.
Member
Why should they be exported? |
grigasp
reviewed
Oct 9, 2026
…ternalStore for the default units provider - startup shares one in-flight initialization and allows a retry after failure; drop the generation guard and late-registration cleanup - useUnitsProvider reads the app provider through useSyncExternalStore, so removing an explicit provider never renders a stale default
Contributor
Author
They were already part of the public signatures (FormatSelector: React.FC, etc.), but consumers couldn’t import them by name. API Extractor listed them as non-exported. So exporting them matches property-grid, which does exports its component props types. I could revert it, but I figured it could help? |
Member
Consumers can import components, which is what's generally used directly. Component props are relatively rarely used directly, so, IMO, there's no point in exporting them. And if consumers do need props, React provides an utility for that -
Not sure what you mean. The
The APIs listed in the two outputs don't have to be the same.
Property grid is not the best example, we're going to clean it up with the next major release. Not exporting component props has been a pattern for all new-generation packages, though, including StrataKit packages (example), AppUI (example), Presentation (example).
FWIW, I think it does more harm than good. When you import from barrel, you expect to get APIs that you're likely to use. Exporting every publicly accessible API through it pollutes barrel exports in a way that makes it impossible to know what you're supposed to use directly. |
Consumers can use React.ComponentProps<typeof Component>; newer iTwin packages keep props types out of the barrel.
Contributor
Author
Gotcha, I've reverted the props exports from the PR now |
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.
TL;DR
The package had cleanup items from the April refactor backlog that affect consumers:
startup()calls could race.unitsProviderand track changes to it themselves.This PR fixes those, makes
unitsProvideroptional (it defaults toIModelApp.quantityFormatter.unitsProvider), and turns on unused-code checks. Minor release.Stacked on #1827.
What
QuantityFormatting.startup/terminatestartup()calls registered the namespace twice. Calls made while startup runs now share it, a failed startup can be retried, and readinglocalizationbefore startup throws a clear error.unitsProvideroptional onQuantityFormatPanel/FormatPanel/FormatSampleIModelApp.quantityFormatter.unitsProviderand re-subscribe toonUnitsProviderChangedby hand. The components now do that by default; an explicit provider still wins. Supported core stays at^5.3.internal/useUnitsProvider.ts(new)useSyncExternalStore.@itwin/core-common,@itwin/ecschema-metadata(^5.3.0)QuantityFormatPanelonFormatChangefires.FormatSampleFormatSetSelectorFormatSetPaneltsconfig.jsonnoUnusedLocals/noUnusedParametersunitsProvider.Tests
New and updated test coverage
QuantityFormatting.test.ts— concurrent startupstartup()is called in parallel.QuantityFormatting.test.ts— localization before startupQuantityFormatting.test.ts— retry after failureuseUnitsProvider.test.ts— default / follows app / explicit wins / explicit removedonUnitsProviderChanged, and no stale default after an explicit provider is removed.FormatSetSelector.test.tsx— stable keys while filteringQuantityFormatPanel.test.tsx— persistence unit reload / loggingQuantityFormatPanel.test.tsx— prop change resets editsformatDefinitiondiscards pending edits without saving.Validation
tsc,pnpm lint,pnpm test(213/213),pnpm build,extract-api,check-extractions, the test-viewer type-check, and thelearning-snippetsquantity-formatting tests all pass.Nambot 🤖 (powered by claude-opus-5.5)