luckyframework / luckyframework/avram
Look in to moving away from monkey-patched core types
- Dominant language
- Crystal
- Stars
- 183
- Forks
- 67
- PR merge metrics
- No merged PRs in 30d
Description
We patch all of the basic types that Avram supports like this:
https://github.com/luckyframework/avram/blob/6922463f2043a727c43ff203ae32f02421c002d8/src/avram/charms/bool_extensions.cr#L1-L6
This is used in a lot of different places like
https://github.com/luckyframework/avram/blob/6922463f2043a727c43ff203ae32f02421c002d8/src/avram/criteria.cr#L121
https://github.com/luckyframework/avram/blob/6922463f2043a727c43ff203ae32f02421c002d8/src/avram/add_column_attributes.cr#L66
https://github.com/luckyframework/avram/blob/6922463f2043a727c43ff203ae32f02421c002d8/src/avram/base_query_template.cr#L13
The idea is that your column specifies a type like `admin : Bool`, then we call `Bool.adapter.whatever` internally. I believe the original intention was to have a swappable adapter to handle doing different things, but we never got around to that.
The other bit is this ensures that Bool related methods are handled by Bool and String related methods are handled by String which gives Lucky (and Avram) an extra layer of type-safety without having to fight the compiler.
There are some downsides to this, and the most recent is that the new `crystal tool unreachable` doesn't work with Lucky at all, and it may boil down to [this](https://github.com/crystal-lang/crystal/issues/14034).
Another issue is that it makes creating custom types a lot [more involved](https://github.com/luckyframework/avram/issues/497).
I think we can come up with a better interface for this which will also alleviate monkey-patching the stdlib types
Contributor guide
Research direction
Start by reviewing src/avram/charms/bool_extensions.cr and its usages in src/avram/criteria.cr, src/avram/add_column_attributes.cr, and src/avram/base_query_template.cr. Compare the current adapter calls with the concerns around crystal tool unreachable and custom types. Done means a decided replacement interface that removes the core-type patches while preserving the stated type-safety behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- crystal
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100