Start passing in an option to ActiveRecordColumns compiler for how to generate column types
#1888
Conversation
This file contains 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
paracycle
force-pushed
the
uk-at-compiler-options
branch
from
4dc1368
to
416a1d1
Compare
KaanOzkan
reviewed
May 2, 2024
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
This is either 'untyped', 'nilable', or 'persisted', with the old behaviour being approximated by 'persisted' Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
All the tests that were testing existence of StrongTypeGeneration but with the model not extending it are now actually testing the "untyped" option for the `column_types` compiler option to AR columns compiler. Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
Co-authored-by: Alexandre Terrasa <583144+Morriar@users.noreply.github.com>
paracycle
force-pushed
the
uk-at-compiler-options
branch
from
416a1d1
to
aa324a6
Compare
Since this option cannot be supplied properly on the command line, we should hide it as a command line option, but support it in the configuration file. We also error out if the option as supplied does not have proper hashes as values.
paracycle
force-pushed
the
uk-at-compiler-options
branch
from
9f20e4b
to
0da147c
Compare
Instead of erroring out when the compiler options are not supplied as hashes, we could try to first parse them as YAML (if they are supplied as strings) and only then error if any of the compiler options isn't a hash.
This allows users to supply compiler options as strings in the `tapioca` CLI as:
```
dsl Post --compiler-options='ActiveRecordColumns:{types: untyped}'
```
which is more user-friendly than not being able to pass these options on the command line.
KaanOzkan
approved these changes
Jun 4, 2024
Agreed. I am happy to do that in a follow up PR. |
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.
Motivation
One of the most painful parts of Tapioca has historically been the types generated for Active Record columns.
Strictly speaking, an AR model will always have all of its column attributes as
nilable, since one can always doModel.newand validations actually behave on this fact being true. However, treating every column attribute as nilable is painful in user code and requires a bunch ofT.musts to be added depending on user intent. For that reason, most developers actually prefer to have the database nullability define the nilable status of a column, since most of the time models are dehydrated from database rows.As an in-between measure, it is also possible to define all column attributes to be
T.untypedso that one can defer dealing with what the exact types should be to a later date (which is what we do at Shopify in our Core monolith - except for a few models that include a special module calledStrongTypeGeneration).Until now, there was no way to choose between these options, and this PR is aiming to add this as a compiler option that can be different for each codebase.
/cc @KaanOzkan
Implementation
The implementation is backwards compatible with existing DSL compiler implementations and only adds an extra optional parameter to the constructor of each DSL compiler named
options. Each compiler is responsible for declaring what options it supports and with which values.This configuration is injected from the CLI/config layer through a hash valued CLI option
compiler_options, which takes in compiler names as keys and config options for the compiler as values.ActiveRecordColumnscompiler declares a single option namedtypeswhich can have the values:untyped: which generates all column attributes asT.untyped,persisted: which generates all column attributes as nilable only if the database column is marked nullable,nilable: which generates all column attributes as nilable (as they should be)The default behaviour of this option is set as
persistedso that the behaviour of this compiler does not change drastically.Tests
Updated existing compiler tests and added a CLI test to ensure that options are passed correctly to
ActiveRecordColumnscompiler.