mobxjs / mst-gql

Bindings for mobx-state-tree and GraphQL
MIT License
684 stars 81 forks source link

dontRenameModels ignored in cosmiconfig config #292

Open barbalex opened 3 years ago

barbalex commented 3 years ago

I tried to use a config file, see here: https://github.com/barbalex/vermehrung/blob/watermelondb/mst-gql.config.js

The reason this would be nice is that my excludes list is so large, it would error the cli call.

The nice part was: I could enlarge the excludes list 😄

Unfortunately, the dontRenameModels option was ignored (https://github.com/barbalex/vermehrung/blob/watermelondb/mst-gql.config.js#L4), so no cigar 😞

barbalex commented 3 years ago

This seems to happen because the dontRenameModels argument is treated differently than most other args in the relevant code, see here: https://github.com/mobxjs/mst-gql/blob/main/generator/config.js#L51-L53, see the difference with noReact:

noReact: !!args["--noReact"] || config.noReact,
namingConvention: args["--dontRenameModels"]
  ? "asis"
  : config.namingConvention,

So in most cases, we can use the same arguments in a cosmiconfig. But not in this one. That seems bad api design.

Would you accept a pull request changing it to be:

{
  namingConvention: args["--dontRenameModels"]
    ? "asis"
    : !!config.namingConvention
    ? config.namingConvention
    : config.dontRenameModels
    ? "asis"
    : undefined
}

This would: