Remove usage of Union(String, Symbol)

[jwoertink]

It's known within Crystal that for each Union you add, there's going to be a small increase in compile-time. I believe this is basically because the compiler has to check each type in the union across the whole app to make sure things are being used correctly.

As apps get larger, compile-time is going to increase, and this is definitely a pain point. While Lucky can't really reduce these times, I think we may be able to make a conscious effort to keep them down as low as we can. I had a thought about our usage of Symbol. I think it's mostly in Avram, but I would be curious to see if we take every single spot that we do thing : String | Symbol, and just make them ALL String, would that cut compile-time down any? 🤔 I'm sure we're talking milliseconds, but is it like 5ms, or could it be like 500ms? 🤷‍♂️

Do you think it would be worth putting in this effort even if it only helps a tiny bit? Or is it better to have the flexibility of doing something like

drop :table

table :users do
end

params.get?(:q)
param[:q]

If we made this change, it would be a breaking change. If people are down for this, I can try it out on one of my longer compiling apps and benchmark the difference

[jwoertink]

Here's a list of some spots where this happens:

lucky/src/charms/hash_extensions.cr

Line 14 in b7d3585

def get(key : String | Symbol)

lucky/src/lucky/params.cr

Line 140 in b7d3585

def get_raw?(key : String | Symbol) : String?

https://github.com/luckyframework/avram/blob/ac949e463af90fd516e845e75744117c72ad4435/src/avram/order_by_clause/order_by.cr#L20
https://github.com/luckyframework/avram/blob/9e36747832a1f387c290ba3e61d4b1963148062e/src/avram/i18n_backend.cr#L6
https://github.com/luckyframework/avram/blob/ac949e463af90fd516e845e75744117c72ad4435/src/avram/migrator/create_index_statement.cr#L33
https://github.com/luckyframework/avram/blob/ac949e463af90fd516e845e75744117c72ad4435/src/avram.cr#L36
https://github.com/luckyframework/avram/blob/ac949e463af90fd516e845e75744117c72ad4435/src/avram/query_builder.cr#L4

After digging through some of this, I'm not sure if we would need to remove all Symbol usage... just the spots where we do a Union(String, Symbol) which would be replaced by String.

[matthewmcgarvey]

While compilation speed would be a real helpful thing to improve I'm not sure the changing those specific places is going to be a big enough payoff to outweigh the downside of inconvenience. I'm wondering if filling in places that are missing explicit type declarations would be helpful.