Repository navigation
Keep generated Dict and SeqDict migrations valid when nested - #111
Open
CharlonTank wants to merge 1 commit into
Open
CharlonTank wants to merge 1 commit into
CharlonTank wants to merge 1 commit into
Conversation
Value migrations were emitted as `Dict.map (\k -> m)`: a value that is itself a migrated Dict produced `Dict.map (\k -> Dict.map (\k -> m))`, rejected by Elm (SHADOWING). Key migrations were emitted as the pipeline `Dict.toList |> List.map (..) |> Dict.fromList`, which is not a function once nested (e.g. `Dict String (SeqDict CustomKey v)`), so the migration failed to type check. Use `\_` and compose with `>>` like the Result template does; this stays valid at the top level (`old.x |> Dict.toList >> ..`).
CharlonTank
added a commit
to CharlonTank/compiler
that referenced
this pull request
Sep 25, 2026
… nested Same templates as Dict and SeqDict, same two problems (see lamdera#111): a nested value migration shadowed its `\k`, and a key migration emitted as a `|>` pipeline is not a function once nested. Use `\_` and compose with `>>`.
3 tasks
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.
Problem
Two of the
Dict/SeqDictmigration templates are only valid at the top level of a field migration:Value migrations are emitted as
Dict.map (\k -> <m>). When the value is itself aDict/SeqDictthat needs a migration, the output iswhich Elm rejects with a SHADOWING error. The existing
Migrate_All/Expected.elmsnapshot already contains exactly this (nestedDictCustomType); it just never gets compiled.Key migrations are emitted as a pipeline fragment,
SeqDict.toList |> List.map (…) |> SeqDict.fromList. That works afterold.field |>, but as soon as it is nested in a function position (Dict.map (\k -> …),Maybe.map (…), …) it is no longer a function. For example,Dict String (SeqDict CustomKey v)generates a migration that fails to type check.We hit both in almost every production migration of our app that touches such a field, and had to fix them by hand each time.
Fix
\_instead of\k. The key is never used, so nesting can no longer shadow anything.>>instead of piping with|>, as theResulttemplate already does. At the top level,old.x |> SeqDict.toList >> List.map (…) >> SeqDict.fromListstays valid because>>binds tighter than|>.Tuple.mapFirstargument, asTuple.mapBothalready does, so a compound key migration stays a single argument. elm-format removes the parentheses when they are redundant.Tests
Migrate_Allscenario gets a new field,nestedSeqDictCustomKey : Dict String (SeqDict UserType Int), which addslamdera/containersto the scenario'selm.json. The snapshots are regenerated.migration-scenariosispending, so I ran it locally withtestExamplesonsrc/Migrate_Allonly: ✅ 2 tests passed. TheMigrate_External_Paramedscenario currently throws first, on a Wire3 encoder type error while compiling the scenario itself, which is unrelated to this change.Expected.elmwithlamdera make. Before the change it fails with SHADOWING onnestedDictCustomType. After it, the only errors left are the intentionalUnimplementedplaceholders.Related
(\v__ -> v__ |> …)when it is passed as an argument. This PR fixes it at the source instead: the templates no longer emit|>pipelines, so that wrapper no longer triggers for them. Composition binds no name, so it stays valid at any depth, whereas a\v__wrapper nested inside another one would shadow it. fix: use old type name in Union migration type signatures for renamed types #81 does not touch the first problem (\k). If both land, only theMigrate_Nested_Dictsnapshot of fix: use old type name in Union migration type signatures for renamed types #81 needs regenerating.