Generate per-table Id newtypes (UserId, ProjectId, etc.)#2460
Open
mpscholten wants to merge 16 commits intomasterfrom
Open
Generate per-table Id newtypes (UserId, ProjectId, etc.)#2460mpscholten wants to merge 16 commits intomasterfrom
mpscholten wants to merge 16 commits intomasterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2d846aeed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1065
to
+1066
| [ "instance Fetchable " <> idTypeName <> " " <> modelName <> " where" | ||
| , " type FetchResult " <> idTypeName <> " " <> modelName <> " = " <> modelName |
There was a problem hiding this comment.
compileFetchableIdInstances emits instance heads like instance Fetchable UserId User, but these declarations are written into Generated.ActualTypes.PrimaryKeys (via compilePrimaryKeysModule) where User/Project types are not defined or imported, so generated app code fails to compile with “Not in scope: type constructor” errors for every single-column table; this only passes in statement preview because preview output inlines the model data/type aliases in the same module.
Useful? React with 👍 / 👎.
…olymorphic Id' Change Id' from a newtype to a type family and generate concrete per-table newtypes like UserId, ProjectId in the schema compiler. This gives much better GHC error messages (e.g. "Expected: ProjectId, Got: UserId" instead of "Expected: Id' \"projects\", Got: Id' \"users\""). A bidirectional pattern synonym `Id` preserves full backwards compatibility so existing code using `Id someUUID` as constructor or `(Id x)` as pattern match continues to work unchanged. Key changes: - Id' is now a type family, with per-table type instances - IdNewtype class + Id pattern synonym for backwards-compatible construction - Schema compiler generates newtypes with all necessary instances (Eq, Ord, Show, Hashable, FromField, ToField, JSON, ParamReader, Fetchable, CollectionFetchRelated, HSX.ApplyAttribute, etc.) - Generic instances removed from library modules (Encoders, FromRow, etc.) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mpscholten
force-pushed
the
per-table-id-newtypes
branch
9 times, most recently
from
8a56183 to
5d8a0d8
Compare
# Conflicts: # ihp/IHP/Fetch/Statement.hs
mpscholten
force-pushed
the
per-table-id-newtypes
branch
from
5d8a0d8 to
97a79db
Compare
- Add IsString instances for PostId, TagId, WeirdTagId in ModelFixtures - Add CompositeTaggingPK newtype for composite key tests - Change composite PK type to (UUID, UUID) with proper Id' type instance - Fix unused imports in HasqlEncoderSpec and ViewSupportSpec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…able instance The generic Fetchable (Id' table) instance was removed since Id' is now a type family. Use genericFetchIdOne directly which has the same constraints that DisplayableJob already provides. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The generated PrimaryKeys module derives Hashable for Id newtypes, which requires Data.Hashable to be available. Add hashable to: - defaultImports in schema compiler (so it's available in all generated modules) - app-models cabal template and nix derivation in NixSupport/default.nix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
With Id' as a type family, Show (PrimaryKey table) doesn't imply Show (Id' table) since the Id newtype has its own Show instance. Update all RLS and DataSync constraints accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
With NoImplicitPrelude + CorePrelude, `show` and `error` are not available unqualified in generated code. Derive Show via newtype instead of manual instance, and qualify as Prelude.error/Prelude.show. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Id' is already re-exported from IHP.Prelude, making the explicit import from IHP.ModelSupport.Types redundant. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Id and Id' are already re-exported from IHP.Prelude. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Generated per-table modules need to import these classes to define instances for per-table Id newtypes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test modules generate code that uses Id' type family applications which now need concrete newtypes. Add mkTestIdNewtype helper that generates TypedSqlTestItemId, TypedSqlTestAuthorId etc. with all required instances (Eq, Ord, Show, IsScalar, IdNewtype, IsString, DefaultParamEncoder). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ClassyPrelude's last returns Maybe, not a plain value. Use reverse/pattern match instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
readMay from ClassyPrelude isn't re-exported by IHP.Prelude. Use qualified Text.Read.readMaybe from base instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve conflicts: - NixSupport/default.nix: include both hashable and postgresql-types deps - IHP/Prelude.hs: export both per-table Id types and FieldBit - IHP/Hasql/FromRow.hs: keep PR's removal of generic Id' instances (per-table newtypes handle it) - IHP/Job/Queue.hs, IHP/Job/Runner.hs: take master's sub-module refactoring - IHP/Job/Queue/Result.hs, IHP/Job/Runner/WorkerLoop.hs: add IdNewtype constraints for per-table newtypes - IHP/SchemaCompiler.hs: include both per-table Id imports and statement imports - Test/SchemaCompilerSpec.hs: include both per-table Id instances and FieldBit instances Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
Id'from a polymorphic newtype to a type family with per-table concrete newtypes (UserId,ProjectId, etc.) generated by the schema compilerExpected: ProjectId, Got: UserIdinstead ofExpected: Id' "projects", Got: Id' "users"Idpattern synonym preserves full backwards compatibility — existingId someUUIDconstruction and(Id x)pattern matching works unchangedKey changes
ihp/IHP/ModelSupport/Types.hs:Id'becomes a type family; addsIdNewtypeclass andIdpattern synonymihp-schema-compiler/IHP/SchemaCompiler.hs: Generates per-table newtypes with all necessary instances (Eq, Ord, Show, Hashable, FromField, ToField, JSON, ParamReader, Fetchable, CollectionFetchRelated, HSX.ApplyAttribute, etc.)Id'instances from Encoders, FromRow, PGSimpleCompat, ModelSupport, Param, ViewSupport — replaced by generated per-table instancesihp/IHP/FetchRelated.hs: Adds helper functionscollectionFetchRelatedById/collectionFetchRelatedOrNothingByIdthat generated instances delegate toTest plan
UserIdandId Userresolves correctly🤖 Generated with Claude Code