Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise API #34

Open
jxxcarlson opened this issue Jul 7, 2024 · 3 comments
Open

Revise API #34

jxxcarlson opened this issue Jul 7, 2024 · 3 comments

Comments

@jxxcarlson
Copy link
Owner

jxxcarlson commented Jul 7, 2024

Some ideas on revising the API.

Below are some possible changes presented in a few cases. But whatever we do, we should strive for a coherent API.

Import

We could use the builder pattern even more systematically like this:

Import.config. "Types" [ module "Dict" |> expose ["Dict"]
                       , module "Task"
                       , module "Foo.Bar |> alias "Bar" ]  
                  |> Import.makeRule

Note the change from Import.init to Import.config, which is more descriptive.

Importing a list of unqualified modules can be done via

Import.Config. "Types" [ module "Dict", module "Task", module "Foo.Bar" ]  |> Import.makeRule

One could add to this the construct

Import.unqualified ["Dict", "Task", "Foo.Bar"]

Implementation notes


module : String -> Install.Config
module name = { moduleToImport = name, alias_ = Nothing, exposedValues = Nothing

FieldInTypeAlias

Suppose we change the implementation of our makeRule to

FieldInTypeAlias.makeRule "Types" "FrontendModel" [ "quote: String", "count: Int", "jokes: List String"]

so that the last argument of makeRule is a list. We could, of course, always say

FieldInTypeAlias.makeRule "Types" "FrontendModel" [ "quote: String" ]

to do what we already do with a slight change of syntax.

TypeVariant

Following the suggestions of FieldInTypeAlias and the goal of a coherent API, we could replace the list of rules

    , Install.TypeVariant.makeRule "Types" "BackendMsg" "GotAtmosphericRandomNumbers (Result Http.Error String)"
    , Install.TypeVariant.makeRule "Types" "BackendMsg" "SetLocalUuidStuff (List Int)"
    , Install.TypeVariant.makeRule "Types" "BackendMsg" "GotFastTick Time.Posix"

by the single rule

TypeVariant.makeRule "Types" "BackendMsg" 
    [ "GotAtmosphericRandomNumbers (Result Http.Error String)"
    , "SetLocalUuidStuff (List Int)"
    , "GotFastTick Time.Posix"
  ]

Initializer

Likewise, the list of rules

  , Install.Initializer.makeRule "Backend" "init" "randomAtmosphericNumbers" "Just [ 235880, 700828, 253400, 602641 ]"
  , Install.Initializer.makeRule "Backend" "init" "time" "Time.millisToPosix 0"

could be replaced by the single rule

Initializer.makeRule "Backend" "init" 
  [ "randomAtmosphericNumbers" "Just [ 235880, 700828, 253400, 602641 ]"
  , "time" "Time.millisToPosix 0" 
  ]

Notes

The cumulative effect of these changes would be quite large, .e.g., on the monstrous rule set for magic link authentication. A rough estimate: a reduction from 92 to 11 rules (if we also do something similar for ClauseInCase)

@mateusfpleite
Copy link
Collaborator

Really great suggestions! I think we can start working on them right away. Do you think it's better to make a big PR by changing everything at once or proceed module by module?

@jxxcarlson
Copy link
Owner Author

It might be best to go module by module. If you would like to take the lead on this, I will review, approve, and merge the PRs as they come in.

@jxxcarlson
Copy link
Owner Author

Also: if you would like to select a couple of modules to work on, I can then select a few others so as to work in parallel and speed things up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants