fix(codepush): forward jsxRuntimes through the partial configuration - #17
Merged
Merged
Conversation
`buildPartialConfiguration` builds a `RequiredOrDiscard<AutoInstrumentationConfiguration>` precisely so that a new field cannot be added without this path being updated to carry it. Adding `jsxRuntimes` did exactly that, and the guard caught it - during the declaration build, which the test suite does not run, so it only surfaced when a release tried to publish. Forward the field, and extend the two assertions that enumerate the resulting configuration.
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
1 participant
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.
buildPartialConfigurationin the codepush package builds aRequiredOrDiscard<AutoInstrumentationConfiguration>specifically so that adding a field to that type cannot silently skip this path. AddingjsxRuntimesin #15 did exactly that, and the guard did its job.The catch is when it fired. The type error only appears while building declaration files (
lerna run prepare), whichyarn testdoes not do — so CI stayed green on #15 and the failure only surfaced when the release workflow ranpreparebefore publishing. Nothing was published: all four packages are still absent from npm at 0.1.4, with 0.1.3 serving as the negative control.This forwards the field and extends the two assertions that enumerate the resulting configuration.
Verified locally:
lerna run preparesucceeds for all 9 projects, 882 tests pass, lint reports no errors.Worth considering separately: the
testworkflow could run the declaration build too, so this class of error is caught on the PR rather than at publish time.