Conversation
Member
Author
|
coderabbit |
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughModernizes tooling and upgrades Node/dev-dependencies, replaces legacy ESLint config with a flat Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Evolve as evolve()
participant Validator as validateChangelog()
participant Parser as parseStoredValueWithInitialization()
participant Initializer as initializeFromUnversioned?
participant Migrator as applyChangesets()
Caller->>Evolve: call with (validChangelog, options?, unknownTarget)
Evolve->>Validator: assume ValidChangelog (pre-validated)
Evolve->>Parser: parseStoredValueWithInitialization(unknownTarget)
alt target has _version
Parser-->>Evolve: Right(StoredValue)
else missing _version and initializer provided
Parser->>Initializer: initializeFromUnversioned(input)
Initializer-->>Parser: Either<Error, StoredValueV0>
alt initializer succeeds
Parser-->>Evolve: Right(StoredValueV0)
else fails
Parser-->>Evolve: Left(INITIALIZE_FROM_UNVERSIONED_FAILED_ERROR)
end
else missing _version and no initializer
Parser-->>Evolve: Left(MISSING_VERSION_ERROR)
end
Evolve->>Evolve: check stored version ≤ latest
alt stored version > latest
Evolve-->>Caller: Left(UNSUPPORTED_FUTURE_VERSION_ERROR)
else apply needed changesets
Evolve->>Migrator: applyChangesets(from storedVersion+1 to latest)
Migrator-->>Evolve: Right(updated StoredValue)
Evolve-->>Caller: Right(StoredValue with latest _version)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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 |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/release.yml (1)
27-27:⚠️ Potential issue | 🟡 MinorVerify
--ciflag is passed to Jest correctly.The
--ciand--maxWorkersflags are passed tonpm run, not directly to Jest. npm does not forward these flags to the underlying script unless you use--separator:npm run test -- --ci --maxWorkers=2.Proposed fix
- npm run test --ci --maxWorkers=2 + npm run test -- --ci --maxWorkers=2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml at line 27, The workflow is incorrectly passing Jest flags to npm instead of to the test script; update the release job's test step to forward flags by adding the `--` separator so the command becomes `npm run test -- --ci --maxWorkers=2`, ensuring `--ci` and `--maxWorkers` are passed to Jest (change the line that currently reads `npm run test --ci --maxWorkers=2`)..github/workflows/ci.yml (1)
40-40:⚠️ Potential issue | 🟡 MinorSame issue: flags not forwarded to Jest.
Proposed fix
- - run: npm run test --ci --coverage --maxWorkers=2 + - run: npm run test -- --ci --coverage --maxWorkers=2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 40, The CI job is invoking "npm run test --ci --coverage --maxWorkers=2" which does not forward those flags to Jest; update the run step to forward arguments by inserting "--" so it becomes "npm run test -- --ci --coverage --maxWorkers=2" (or alternatively call Jest directly, e.g., "npx jest --ci --coverage --maxWorkers=2") to ensure the --ci, --coverage and --maxWorkers flags reach the test runner.src/core/Evolutions.ts (1)
49-82:⚠️ Potential issue | 🟠 MajorAdd runtime validation that changesets produce JSON objects before treating them as
StoredValue.Both
applyPatch(...).newDocumentandupdate(t, cs.spec)can return primitives or non-object values. When this occurs, the subsequent{...result, _version: latest}spread silently fabricates an object instead of rejecting the invalid state. This bypasses the stored-value invariant.Use
isRecord(result)in anE.chainto validate the result is an object before treating it as aStoredValue. Apply this to both the JSON_PATCH_CHANGESET and IMMUTABILITY_HELPER_CHANGESET cases.💡 Suggested hardening
case "JSON_PATCH_CHANGESET": - return E.tryCatch( - () => applyPatch(t, cs.patch, true, false).newDocument, - (e) => - e instanceof JsonPatchError - ? { - errorCode: "JSON_PATCH_EVOLUTION_ERROR", - message: `Failed to apply JSON patch changeset with version ${cs._version}: ${e.message}`, - error: e, - } - : { - errorCode: "UNEXPECTED_EVOLUTION_ERROR", - message: `Unexpected JSON patch error: ${String( - e, - )}`, - }, - ); + return pipe( + E.tryCatch( + () => applyPatch(t, cs.patch, true, false).newDocument, + (e) => + e instanceof JsonPatchError + ? { + errorCode: "JSON_PATCH_EVOLUTION_ERROR", + message: `Failed to apply JSON patch changeset with version ${cs._version}: ${e.message}`, + error: e, + } + : { + errorCode: "UNEXPECTED_EVOLUTION_ERROR", + message: `Unexpected JSON patch error: ${String( + e, + )}`, + }, + ), + E.chain((result) => + isRecord(result) + ? E.right(result as StoredValue) + : E.left({ + errorCode: "INVALID_STORED_VALUE_ERROR", + message: `Changeset version ${cs._version} must produce a JSON object.`, + }), + ), + ); case "IMMUTABILITY_HELPER_CHANGESET": - return E.tryCatch( - () => update(t, cs.spec) as StoredValue, - (e) => - e instanceof Error - ? { - errorCode: - "IMMUTABILITY_HELPER_EVOLUTION_ERROR", - message: `Failed to apply Immutability helper changeset with version ${cs._version}: ${e.message}`, - error: e, - } - : { - errorCode: "UNEXPECTED_EVOLUTION_ERROR", - message: `Unexpected immutability-helper error: ${String( - e, - )}`, - }, - ); + return pipe( + E.tryCatch( + () => update(t, cs.spec), + (e) => + e instanceof Error + ? { + errorCode: + "IMMUTABILITY_HELPER_EVOLUTION_ERROR", + message: `Failed to apply Immutability helper changeset with version ${cs._version}: ${e.message}`, + error: e, + } + : { + errorCode: "UNEXPECTED_EVOLUTION_ERROR", + message: `Unexpected immutability-helper error: ${String( + e, + )}`, + }, + ), + E.chain((result) => + isRecord(result) + ? E.right(result as StoredValue) + : E.left({ + errorCode: "INVALID_STORED_VALUE_ERROR", + message: `Changeset version ${cs._version} must produce a JSON object.`, + }), + ), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/Evolutions.ts` around lines 49 - 82, The JSON_PATCH_CHANGESET and IMMUTABILITY_HELPER_CHANGESET branches currently return raw results from E.tryCatch (applyPatch(...).newDocument and update(t, cs.spec)) which may be primitives; add an E.chain after each E.tryCatch to validate the result with isRecord(result) and reject with a suitable error object if it is not an object, otherwise cast/return it as StoredValue; update the branches that reference applyPatch(...).newDocument and update(t, cs.spec) to use E.tryCatch(...).chain(result => isRecord(result) ? E.right(result as StoredValue) : E.left({ errorCode: "INVALID_STORED_VALUE", message: `Changeset version ${cs._version} did not produce an object`, original: result })); ensure this validation is applied for both JSON_PATCH_CHANGESET and IMMUTABILITY_HELPER_CHANGESET.
🧹 Nitpick comments (2)
eslint.config.mjs (1)
17-46: Consider using typescript-eslint's flat config helpers.The manual rule composition accessing
tsPlugin.configs["eslint-recommended"].overrides?.[0]?.rulesis fragile as it relies on the internal structure. Thetypescript-eslintpackage v8+ (currently installed as v8.43.0) exports flat config helpers that provide a more robust alternative while maintaining type-aware linting viaprojectService. The optional chaining provides a safe fallback, so the current setup works, but the refactoring would simplify the configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.mjs` around lines 17 - 46, Replace the fragile manual rule composition that references tsPlugin.configs["eslint-recommended"].overrides?.[0]?.rules with typescript-eslint's flat config helper getESLintFlatConfigForTypeScript (exported from the package) to produce the TypeScript-aware ruleset; call getESLintFlatConfigForTypeScript with the same parserOptions (project: "./tsconfig.eslint.json", tsconfigRootDir: import.meta.dirname) and then merge its returned config into your exported array/object alongside your files filter, languageOptions (parser: tsParser), plugins, and custom rules like "no-console" and "@typescript-eslint/no-empty-object-type"; this removes the brittle tsPlugin.configs access and keeps the same type-checked linting via projectService.src/core/Evolutions.ts (1)
177-177: Freeze the sorted changelog before branding it.
ValidChangelogis readonly only at the type level today; in plain JS the returned array can still be mutated after validation and break the invariant thatevolve()trusts. Freezing the sorted copy would make the runtime contract match the API.💡 Suggested hardening
- return E.right((sorted as unknown) as ValidChangelog); + return E.right( + Object.freeze(sorted) as unknown as ValidChangelog, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/Evolutions.ts` at line 177, The returned sorted changelog is only readonly at the TS type level but remains mutable at runtime; before branding and returning it as a ValidChangelog in the function that currently returns E.right((sorted as unknown) as ValidChangelog), freeze the runtime structure (e.g., call Object.freeze on the sorted array and, if entries are plain objects, freeze each entry as well) so evolve() and other consumers cannot mutate it; update the return to brand and return the frozen object (use the existing sorted variable and the ValidChangelog cast).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 23: The npm run invocation is not forwarding flags to the underlying Jest
script; update the CI job command that currently reads the run step invoking
"npm run test --ci --coverage --maxWorkers=2" to include the "--" separator so
the flags are passed through to Jest (i.e., change the run command to use "npm
run test -- --ci --coverage --maxWorkers=2").
In `@src/core/Evolutions.ts`:
- Around line 123-130: The call to initializeFromUnversioned(input) can throw
synchronously and escape the Either flow; wrap the invocation in an Either-safe
try/catch (e.g., fp-ts E.tryCatch or an explicit try/catch that returns a Left)
so thrown exceptions become a Left and then map that Left to the same shape used
by the existing mapLeft block; update the pipe around
initializeFromUnversioned(input) (the call used before E.mapLeft and
E.chain(ensureInitializedValue)) to use the try-catch wrapper so all thrown
errors are converted to an initializeError and handled via the existing
INITIALIZE_FROM_UNVERSIONED_FAILED_ERROR mapping.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Line 40: The CI job is invoking "npm run test --ci --coverage --maxWorkers=2"
which does not forward those flags to Jest; update the run step to forward
arguments by inserting "--" so it becomes "npm run test -- --ci --coverage
--maxWorkers=2" (or alternatively call Jest directly, e.g., "npx jest --ci
--coverage --maxWorkers=2") to ensure the --ci, --coverage and --maxWorkers
flags reach the test runner.
In @.github/workflows/release.yml:
- Line 27: The workflow is incorrectly passing Jest flags to npm instead of to
the test script; update the release job's test step to forward flags by adding
the `--` separator so the command becomes `npm run test -- --ci --maxWorkers=2`,
ensuring `--ci` and `--maxWorkers` are passed to Jest (change the line that
currently reads `npm run test --ci --maxWorkers=2`).
In `@src/core/Evolutions.ts`:
- Around line 49-82: The JSON_PATCH_CHANGESET and IMMUTABILITY_HELPER_CHANGESET
branches currently return raw results from E.tryCatch
(applyPatch(...).newDocument and update(t, cs.spec)) which may be primitives;
add an E.chain after each E.tryCatch to validate the result with
isRecord(result) and reject with a suitable error object if it is not an object,
otherwise cast/return it as StoredValue; update the branches that reference
applyPatch(...).newDocument and update(t, cs.spec) to use
E.tryCatch(...).chain(result => isRecord(result) ? E.right(result as
StoredValue) : E.left({ errorCode: "INVALID_STORED_VALUE", message: `Changeset
version ${cs._version} did not produce an object`, original: result })); ensure
this validation is applied for both JSON_PATCH_CHANGESET and
IMMUTABILITY_HELPER_CHANGESET.
---
Nitpick comments:
In `@eslint.config.mjs`:
- Around line 17-46: Replace the fragile manual rule composition that references
tsPlugin.configs["eslint-recommended"].overrides?.[0]?.rules with
typescript-eslint's flat config helper getESLintFlatConfigForTypeScript
(exported from the package) to produce the TypeScript-aware ruleset; call
getESLintFlatConfigForTypeScript with the same parserOptions (project:
"./tsconfig.eslint.json", tsconfigRootDir: import.meta.dirname) and then merge
its returned config into your exported array/object alongside your files filter,
languageOptions (parser: tsParser), plugins, and custom rules like "no-console"
and "@typescript-eslint/no-empty-object-type"; this removes the brittle
tsPlugin.configs access and keeps the same type-checked linting via
projectService.
In `@src/core/Evolutions.ts`:
- Line 177: The returned sorted changelog is only readonly at the TS type level
but remains mutable at runtime; before branding and returning it as a
ValidChangelog in the function that currently returns E.right((sorted as
unknown) as ValidChangelog), freeze the runtime structure (e.g., call
Object.freeze on the sorted array and, if entries are plain objects, freeze each
entry as well) so evolve() and other consumers cannot mutate it; update the
return to brand and return the frozen object (use the existing sorted variable
and the ValidChangelog cast).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6f583c9-4efb-4d9e-83a0-b44c86c49d33
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.eslintrc.json.github/workflows/ci.yml.github/workflows/release.ymlCONTRIBUTING.mdREADME.mdeslint.config.mjsjest.config.jspackage.jsonsrc/api/Changeset.tssrc/api/EvolutionError.tssrc/core/Evolutions.tstest/core/Evolutions.spec.tstest/core/__fixtures__/configuration/v0.tstest/core/__fixtures__/configuration/v1.tstest/core/__fixtures__/configuration/v2.tstsconfig.eslint.jsontsconfig.json
💤 Files with no reviewable changes (2)
- jest.config.js
- .eslintrc.json
Introduce validated changelog creation, strict stored-value parsing, and initializeFromUnversioned for bootstrapping older records. Update the test suite, documentation, and build tooling to match the new migration flow and move the package to the modern TypeScript, ESLint, and Jest stack. BREAKING CHANGE: evolve now expects a validated changelog and rejects missing, invalid, and future _version values by default; createChangelog/validateChangelog and initializeFromUnversioned replace the previous looser behavior, and the package now requires Node 24.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/core/Evolutions.ts (1)
138-145:⚠️ Potential issue | 🟠 MajorKeep
initializeFromUnversionedinside theEitherboundary.
initializeFromUnversioned(input)is still invoked directly. If user code throws synchronously,parseStoredValueWithInitialization()throws instead of returningINITIALIZE_FROM_UNVERSIONED_FAILED_ERROR, soevolve()no longer honors itsEithercontract. Wrap the callback in atry/catchthat converts thrown errors into thatLeftbefore the existing mapping and validation flow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/Evolutions.ts` around lines 138 - 145, The code currently calls initializeFromUnversioned(input) directly so any synchronous throw escapes the Either flow; wrap that call in a try/catch (or use fp-ts/Either.tryCatch) to convert thrown exceptions into a Left with the same shape used later (errorCode: "INITIALIZE_FROM_UNVERSIONED_FAILED_ERROR", message: "Failed to initialize an unversioned value.", error: the thrown error) and then pass that Either into the existing pipe so E.mapLeft and E.chain(ensureInitializedValue) operate inside the Either boundary (i.e., replace the direct initializeFromUnversioned(input) invocation with a try/catch-produced Either before the rest of the pipe).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 79-83: The examples call E.chain(codec.decode) where evolve(...)
returns Either<EvolutionError, A> and codec.decode returns Either<t.Errors, B>,
causing a type mismatch; update the examples (the pipe using evolve and
codec.decode and other occurrences noted) to use E.chainW(codec.decode) or
E.flatMap(codec.decode) instead of E.chain so the left error types are merged
into Either<EvolutionError | t.Errors, B>; replace each E.chain(codec.decode)
instance (including the pipe snippet and the occurrences around the listed
lines) with E.chainW(codec.decode) or E.flatMap(codec.decode).
In `@src/core/Evolutions.ts`:
- Around line 57-98: The switch on cs.type inside applyChangeset (the branch
handling "JSON_PATCH_CHANGESET" and "IMMUTABILITY_HELPER_CHANGESET") is not
exhaustive and can fall through for unknown cs.type values; add a default branch
that returns a Left-like E.left containing an error object with errorCode
"INVALID_CHANGELOG_ERROR" (and a helpful message referencing cs._version /
cs.type) so the function preserves the Either contract instead of returning
undefined—ensure the default mirrors the shape used by other errors and plays
nicely with ensureStoredValueResult and subsequent E.map calls.
---
Duplicate comments:
In `@src/core/Evolutions.ts`:
- Around line 138-145: The code currently calls initializeFromUnversioned(input)
directly so any synchronous throw escapes the Either flow; wrap that call in a
try/catch (or use fp-ts/Either.tryCatch) to convert thrown exceptions into a
Left with the same shape used later (errorCode:
"INITIALIZE_FROM_UNVERSIONED_FAILED_ERROR", message: "Failed to initialize an
unversioned value.", error: the thrown error) and then pass that Either into the
existing pipe so E.mapLeft and E.chain(ensureInitializedValue) operate inside
the Either boundary (i.e., replace the direct initializeFromUnversioned(input)
invocation with a try/catch-produced Either before the rest of the pipe).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6e69945-3a50-41cb-8e69-925864d69fae
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.eslintrc.json.github/workflows/ci.yml.github/workflows/release.ymlCONTRIBUTING.mdREADME.mdeslint.config.mjsjest.config.jspackage.jsonsrc/api/Changeset.tssrc/api/EvolutionError.tssrc/core/Evolutions.tstest/core/Evolutions.spec.tstest/core/__fixtures__/configuration/v0.tstest/core/__fixtures__/configuration/v1.tstest/core/__fixtures__/configuration/v2.tstsconfig.eslint.jsontsconfig.json
💤 Files with no reviewable changes (2)
- jest.config.js
- .eslintrc.json
✅ Files skipped from review due to trivial changes (4)
- tsconfig.json
- tsconfig.eslint.json
- CONTRIBUTING.md
- src/api/EvolutionError.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- .github/workflows/release.yml
- test/core/fixtures/configuration/v1.ts
- eslint.config.mjs
- src/api/Changeset.ts
- test/core/fixtures/configuration/v2.ts
- .github/workflows/ci.yml
- test/core/Evolutions.spec.ts
Comment on lines
79
to
83
| pipe( | ||
| {_version: 0, defaultFields: ["field1", "field2"]}, | ||
| evolve(changelog), | ||
| E.map(codec.decode), | ||
| E.chain(codec.decode), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In fp-ts 2.16.11 and io-ts 2.2.22, what are the TypeScript signatures of Either.chain, Either.chainW, and Either.flatMap? When combining Either<EvolutionError, A> with codec.decode from io-ts (Either<t.Errors, B>), which combinator preserves both left error types?
💡 Result:
fp-ts Either (v2.16.11) signatures
// Either.chain (alias of flatMap)
export declare const chain:
<E, A, B>(f: (a: A) => Either<E, B>) =>
(ma: Either<E, A>) => Either<E, B>[1]
// Either.chainW (alias of flatMap; widens/merges Left types)
export declare const chainW:
<E2, A, B>(f: (a: A) => Either<E2, B>) =>
<E1>(ma: Either<E1, A>) => Either<E2 | E1, B>[2]
// Either.flatMap (data-last + data-first overloads; merges Left types)
export declare const flatMap: {
<A, E2, B>(f: (a: A) => Either<E2, B>):
<E1>(ma: Either<E1, A>) => Either<E2 | E1, B>
<E1, A, E2, B>(ma: Either<E1, A>, f: (a: A) => Either<E2, B>):
Either<E1 | E2, B>
}[3]
io-ts decode (v2.2.22) result type
decode: (i: I) => Either<Errors, A> // where Errors = t.Errors[4]
Which combinator preserves both left error types?
If you have:
Either<EvolutionError, A>andcodec.decode(...) : Either<t.Errors, B>
…the combinator that preserves both Left error types is Either.chainW (equivalently, Either.flatMap), yielding:
Either<EvolutionError | t.Errors, B>.
Using plain Either.chain forces the same E on both sides, so it won’t keep EvolutionError and t.Errors as a union. [1][2][3]
Sources:
[1] (gcanti.github.io)
[2] (gcanti.github.io)
[3] (gcanti.github.io)
[4] (jsdocs.io)
Citations:
- 1: https://gcanti.github.io/fp-ts/modules/Either.ts.html
- 2: https://gcanti.github.io/fp-ts/modules/Either.ts.html
- 3: https://gcanti.github.io/fp-ts/modules/Either.ts.html
- 4: https://www.jsdocs.io/package/io-ts
🏁 Script executed:
# Check README.md exists and examine the specific lines mentioned
file_lines=$(wc -l < README.md)
echo "README.md has $file_lines lines"
# Show lines 79-83
echo "=== Lines 79-83 ==="
sed -n '79,83p' README.md
# Show line 88
echo "=== Line 88 ==="
sed -n '88p' README.md
# Show lines 151-155
echo "=== Lines 151-155 ==="
sed -n '151,155p' README.md
# Search for all occurrences of E.chain in README
echo "=== All E.chain occurrences ==="
grep -n "E\.chain" README.md || echo "No E.chain found"Repository: toolsplus/json-evolutions
Length of output: 866
Replace E.chain(codec.decode) with E.chainW(codec.decode) or E.flatMap(codec.decode) in code examples.
The examples use E.chain(codec.decode) where evolve() returns Either<EvolutionError, A> and codec.decode returns Either<t.Errors, B>. In fp-ts 2.16.11, Either.chain requires the left type parameter E to be identical on both sides, so this won't type-check. Use Either.chainW or Either.flatMap instead, which merge the left types into a union: Either<EvolutionError | t.Errors, B>.
Also applies to: lines 88, 151-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 79 - 83, The examples call E.chain(codec.decode)
where evolve(...) returns Either<EvolutionError, A> and codec.decode returns
Either<t.Errors, B>, causing a type mismatch; update the examples (the pipe
using evolve and codec.decode and other occurrences noted) to use
E.chainW(codec.decode) or E.flatMap(codec.decode) instead of E.chain so the left
error types are merged into Either<EvolutionError | t.Errors, B>; replace each
E.chain(codec.decode) instance (including the pipe snippet and the occurrences
around the listed lines) with E.chainW(codec.decode) or E.flatMap(codec.decode).
Comment on lines
57
to
98
| switch (cs.type) { | ||
| case "JSON_PATCH_CHANGESET": | ||
| return E.tryCatch( | ||
| () => applyPatch(t, cs.patch, true).newDocument, | ||
| (e) => | ||
| e instanceof JsonPatchError | ||
| ? { | ||
| errorCode: "JSON_PATCH_EVOLUTION_ERROR", | ||
| message: `Failed to apply JSON patch changeset with version ${cs._version}: ${e.message}`, | ||
| error: e, | ||
| } | ||
| : { | ||
| errorCode: "UNEXPECTED_EVOLUTION_ERROR", | ||
| message: `Unexpected JSON patch error: ${e}`, | ||
| }, | ||
| return pipe( | ||
| E.tryCatch( | ||
| () => applyPatch(t, cs.patch, true, false).newDocument, | ||
| (e) => | ||
| e instanceof JsonPatchError | ||
| ? { | ||
| errorCode: "JSON_PATCH_EVOLUTION_ERROR" as const, | ||
| message: `Failed to apply JSON patch changeset with version ${cs._version}: ${e.message}`, | ||
| error: e, | ||
| } | ||
| : { | ||
| errorCode: "UNEXPECTED_EVOLUTION_ERROR" as const, | ||
| message: `Unexpected JSON patch error: ${String( | ||
| e, | ||
| )}`, | ||
| }, | ||
| ), | ||
| E.chain(ensureStoredValueResult(cs._version)), | ||
| ); | ||
| case "IMMUTABILITY_HELPER_CHANGESET": | ||
| return E.tryCatch( | ||
| () => update(t, cs.spec), | ||
| (e) => | ||
| e instanceof Error | ||
| ? { | ||
| errorCode: | ||
| "IMMUTABILITY_HELPER_EVOLUTION_ERROR", | ||
| message: `Failed to apply Immutability helper changeset with version ${cs._version}: ${e.message}`, | ||
| error: e, | ||
| } | ||
| : { | ||
| errorCode: "UNEXPECTED_EVOLUTION_ERROR", | ||
| message: `Unexpected immutability-helper error: ${e}`, | ||
| }, | ||
| return pipe( | ||
| E.tryCatch( | ||
| () => update(t, cs.spec) as unknown, | ||
| (e) => | ||
| e instanceof Error | ||
| ? { | ||
| errorCode: "IMMUTABILITY_HELPER_EVOLUTION_ERROR" as const, | ||
| message: `Failed to apply Immutability helper changeset with version ${cs._version}: ${e.message}`, | ||
| error: e, | ||
| } | ||
| : { | ||
| errorCode: "UNEXPECTED_EVOLUTION_ERROR" as const, | ||
| message: `Unexpected immutability-helper error: ${String( | ||
| e, | ||
| )}`, | ||
| }, | ||
| ), | ||
| E.chain(ensureStoredValueResult(cs._version)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Return a Left for unsupported changeset types.
This switch is only exhaustive for typed callers. Because validateChangelog() currently brands based on version checks alone, a malformed changelog from JS or an unsafe cast can still reach this branch; in that case applyChangeset() falls off the end and the next E.map throws on undefined instead of preserving the Either contract. Add a default branch that returns INVALID_CHANGELOG_ERROR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/Evolutions.ts` around lines 57 - 98, The switch on cs.type inside
applyChangeset (the branch handling "JSON_PATCH_CHANGESET" and
"IMMUTABILITY_HELPER_CHANGESET") is not exhaustive and can fall through for
unknown cs.type values; add a default branch that returns a Left-like E.left
containing an error object with errorCode "INVALID_CHANGELOG_ERROR" (and a
helpful message referencing cs._version / cs.type) so the function preserves the
Either contract instead of returning undefined—ensure the default mirrors the
shape used by other errors and plays nicely with ensureStoredValueResult and
subsequent E.map calls.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Introduce validated changelog creation, strict stored-value parsing, and initializeFromUnversioned for bootstrapping older records. Update the test suite, documentation, and build tooling to match the new migration flow and move the package to the modern TypeScript, ESLint, and Jest stack.
BREAKING CHANGE: evolve now expects a validated changelog and rejects missing, invalid, and future _version values by default; createChangelog/validateChangelog and initializeFromUnversioned replace the previous looser behavior, and the package now requires Node 24.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests