Don't automatically run core-quantity's generateUnitsJson.ts at build time

[tcobbs-bentley]

Use case

Currently, the generateUnitsJson.ts script in core/quantity is run during every build, regenerating a number of git-tracked files:

• core/quantity/src/assets/Units.json
• core/quantity/src/generated/Units.generated.ts
• core/quantity/src/internal/BasicUnitConversions.generated.ts
• core/quantity/src/internal/DefaultPersistenceUnits.generated.ts

If changes are made by a developer that affect that generation, it will result in one or more of those files being changed in the developer's source tree, with no obvious reason as to why they were changed, leading to likely confusion. Furthermore, the developer may not know if the changes are expected and correct or due to an error in their tree.

Proposed solution

In my opinion, the following changes should be made:

1. Do not run this script automatically as part of the build.
2. Update the script to take an optional destination folder as input. It will then generate files in the given destination.
3. Add a unit test that runs the script with a temporary destination and then compares the results to the original git-tracked files. If they differ, the unit test will fail with an error message instructing the developer to determine if their changes were expected to cause such changes. If so, they can run the script without the destination folder argument to updated the git-tracked files. If not, they can fix whatever caused the undesired changes.

Alternatives considered

1. Leave things like they are now. I don't feel this is a good idea.
2. Update the generation script to add a comment to the top of each generated file instructing developers what to do if the file contents change in their branch. (This is an easier solution, but I feel inferior to the proposed solution. It also won't work for the JSON file since JSON doesn't support comments.)

Willingness to contribute

• I am willing to write the code or documentation for this feature.

AI acknowledgment

• I used AI to generate this feature request.
• (If the above is checked) I have reviewed the AI-generated content before submitting.

[GytisCepk]

If changes are made by a developer that affect that generation, it will result in one or more of those files being changed in the developer's source tree, with no obvious reason as to why they were changed, leading to likely confusion.

I don't understand this part. If developer's changes result in regeneration of those files, changes should be reflected in the source tree and developer should understand why they changed. If developer's changes should not have affected these files, but files were regenerated, that sounds like a bug in generation script.

In my experience with other code generation tools (e.g. protobuf), you keep generated code / assets in the repository so developer and reviewers could easily see how published package will change.

2. Update the script to take an optional destination folder as input. It will then generate files in the given destination.

I think, this makes sense. Generate assets should be in separate directory to clearly distinguish what should be edited by hand and what should be left for generation.

[tcobbs-bentley]

If changes are made by a developer that affect that generation, it will result in one or more of those files being changed in the developer's source tree, with no obvious reason as to why they were changed, leading to likely confusion.

I don't understand this part. If developer's changes result in regeneration of those files, changes should be reflected in the source tree and developer should understand why they changed. If developer's changes should not have affected these files, but files were regenerated, that sounds like a bug in generation script.

I can easily see a developer making a change without knowing that that change will update these files, but I could be wrong about that. Remember, the more time passes, the more people forget about why old mechanisms do what they do, and the more people aren't even aware that the mechanisms exist.

In my experience with other code generation tools (e.g. protobuf), you keep generated code / assets in the repository so developer and reviewers could easily see how published package will change.

I'm not suggesting that we remove the code from source control. I'm only suggesting that it should only be generated on demand (with a new unit test that fails if that is required). (I could have included removing the code from source control in my list of alternatives considered, but ultimately decided that that wasn't a good idea.)

See:

• core/common/src/geometry/ElementGeometryFB.ts
• core/geometry/src/serialization/BGFBAccessors.ts
• core/orbitgt/src/pointcloud/render/TileLoadSorter.ts

[aruniverse]

The concern here is valid: core/quantity is the only package in the monorepo whose build step rewrites git-tracked source files. Every other package's build is strictly compile (plus maybe copy-assets). Someone who runs rush build after touching anything unit-conversion-adjacent, sees four modified files in git status, and has no idea whether to commit them or throw them away — that's a genuine DX problem worth fixing.

That said, a few things are worth calling out before anyone starts implementing.

Point 3 is already solved. GenerateUnitsArtifacts.test.ts already exists on master with four byte-identity tests that rebuild every artifact in memory and assert exact equality with the checked-in files. They catch any drift between the generator and the checked-in outputs. The "optional destination folder" approach described in the proposal would be more complex (temp-dir creation, file writes from tests, cleanup) with no benefit over what's already there — pure in-memory comparison is strictly simpler.

The optional destination folder (Point 2) can be dropped. The existing test design already does what the proposal is trying to achieve. There's no need to add CLI argument parsing to the script.

The root gap is CI, not the build step. Removing the generator from build without adding a CI drift check doesn't actually solve the problem — it just changes "files silently update during build" to "files silently drift until someone runs tests locally." CI doesn't run rush test, so test-based enforcement is developer-side only. The right model is the one already used for core/extension: a separate generate step, the output committed to git, and a git status --porcelain check in CI that fails the build if the checked-in files are out of date. Without that gate, a PR that bumps @bentley/units-schema can still merge with silently stale artifacts — just now with no self-healing mechanism at all.

Suggested implementation path:

1. Add "generate": "tsx scripts/generateUnitsJson.ts" to package.json and remove the generator prefix from build
2. Add a CI step (mirroring the existing extension-file check) that runs rushx generate then asserts git status --porcelain is clean for the four quantity artifacts
3. Update the AUTO-GENERATED comment at the top of each generated file to say how to regenerate (rushx generate)
4. Add a custom failure message to the four toBe() assertions in GenerateUnitsArtifacts.test.ts so a test failure tells the developer what to do rather than dumping a 500-line diff

That covers the real problem without the extra complexity of destination folders or a --check flag.

[hl662]

Alright, fair.

Quick update: I’m putting a small, directly related guardrail into #9443.

That PR already stabilizes the generated unit conversion output by canonicalizing the generated numeric literals, adds tests around the suspicious tiny number changes. This catches the 'oh, its a bug in the generator' scenario.

As for the 'oh something I changed in the source files of quantity led to a change to the autogenerated files' scenario, I also added a CI check in ci.yaml, so rush build fails the workflow if it rewrites the generated unit artifacts and those changes were not already committed by a user (by diffing).

I’m going to keep the broader cleanup as a follow-up PR instead of expanding #9443 too much. That follow-up should cover:

• automated tests for the hook script behavior, especially the staged-tree rush change fallback
• any cleanup/generalization around generated-file drift checks, if we want something less one-off than the current targeted CI guardrail

So #9443 gets the immediate protection for this generated unit output issue, and the follow-up can focus on making the hook/checking workflow more systematically tested without mixing that into the conversion-output fix.