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

Configuration System #297

Open
jasonkuhrt opened this issue Jan 17, 2020 · 2 comments · Fixed by #367
Open

Configuration System #297

jasonkuhrt opened this issue Jan 17, 2020 · 2 comments · Fixed by #367
Labels
impact/high scope/plugins type/feat Add a new capability or enhance an existing one

Comments

@jasonkuhrt
Copy link
Member

jasonkuhrt commented Jan 17, 2020

What

  • Configuration keeps coming up, see related (below) for examples

  • autocompletion and jsdoc should be optimized for

  • so many questions/ways we could do this...

  • function-object approach:

     app.settings({
       server: {
         experiments: {
           middleware: true,
         },
       },
     })		
  • object approach:

     app.settings.server.experiments.middleware = true
     app.settings.server.logger.level = 'debug'
     app.settings.experiments.server.middleware = true
     app.settings.plugins.prisma.crud.relations.createable = false
  • function-object and object approaches could be combined

  • some screenshot / dx observations:

    • vscode gives awkward autocomplete for objects when curly braces are broken across lines.

      image

    • vscode/ts does not give a preview of the settings one can set on param preview (is case for both type = ... and interface = ...)

      Screen Shot 2020-01-16 at 8 30 30 PM
    • user has no autocomplete with object unless they know how to summon the autocomplete with hotkey (ctrl-space). Even if they know, unclear how well that fits into their flow (e.g. they only know how via menu or have awkward bindings or are on a friend's computer with different customizations, ...)

      Screen Shot 2020-01-16 at 8 30 41 PM
    • errors can be visually loud

      Screen Shot 2020-01-16 at 8 51 52 PM
    • error information can be non-trivial

      image

    • working with object properties leads to clean autocomplete compared to above:

      image

  • function-object approach however is JS idiomatic and a succinct experience

  • to keep the mental model simple, app.settings should make its data readable in plain form:

     	console.log('the port is %s', app.settings.server.port)
  • so even with function-object for writes there would still be object for reads

  • at that point, might as well permits writes and leave it up as a style decision? object read/write is the most "basic" but the function-object is there for sugar

about hierarchy

app.settings.<component>.*
app.settings.plugins.<plugin>.*
app.settings.logger.*
app.settings.server.*
app.settings.db.*
app.settings.plugins.prisma.*

Sometimes duplicated read/write fields under different paths might be nice. Take the experiments system for example:

app.settings.<component>.experiments.*
app.settings.experiments.<component>.*

Basically When something could be distributed or centralized, we should do both so that the user doesn't have to remember.

  • settings would need to be applied before e.g. nexus schema
  • because e.g. app.settings.plugins.prisma might affect how nexus-prisma will behave during nexus schema time
  • this starts to feel complex when realizing we're offering up an api for settings where user could do pretty well anything... e.g. they can order their code how they want
  • we'll just have to see if insurmountable issues arise or not
  • absolute worst case would be an e.g. settings.ts convention

Related

Places where need for config has come up:

@jasonkuhrt jasonkuhrt added type/feat Add a new capability or enhance an existing one plugin-system impact/high labels Jan 17, 2020
@jasonkuhrt jasonkuhrt pinned this issue Jan 17, 2020
@jasonkuhrt jasonkuhrt mentioned this issue Jan 30, 2020
@Weakky
Copy link
Collaborator

Weakky commented Feb 3, 2020

A few thoughts:

app.settings.plugins.prisma.crud.relations.createable = false

I find this quite terrible 😢. As much as I understand the reasoning for considering the approach, the level of nesting is just too much if we ever get there. I'd rather have a nested object..

to keep the mental model simple, app.settings should make its data readable in plain form:

We currently use the approach of having app.settings being both a "reader" and a "writer". This raised the drawback of having the function prototype in the autocomplete of app.settings. We could simplify things by having app.settings.set({ ... }) and app.settings.get().*. It's not very elegant, but also still keeps "the mental model simple"

jasonkuhrt added a commit that referenced this issue Feb 4, 2020
jasonkuhrt pushed a commit that referenced this issue Feb 4, 2020
closes #297

BREAKING CHANGE:

- Settings that used to be passed to `server.start` are now under `server` key in settings input.

- `log.settings` no longer exists. Settings that used to be passed to it are now under `logger` key in settings input.

- `schema.settings` no longer exists. Settings that used to be passed to it are now under `schema` key in settings input.
@Weakky Weakky reopened this Feb 4, 2020
@jasonkuhrt jasonkuhrt unpinned this issue Feb 29, 2020
@jasonkuhrt jasonkuhrt pinned this issue Mar 26, 2020
@jasonkuhrt jasonkuhrt changed the title Configuration system Configuration System Mar 26, 2020
@jasonkuhrt
Copy link
Member Author

Take inspiration from https://fastapi.tiangolo.com/advanced/settings/

@jasonkuhrt jasonkuhrt unpinned this issue May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/high scope/plugins type/feat Add a new capability or enhance an existing one
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants