trailsjs / trails

:evergreen_tree: Modern Web Application Framework for Node.js.
http://trailsjs.io
Other
1.66k stars 70 forks source link

V3 new config confusing #312

Closed jaumard closed 6 years ago

jaumard commented 6 years ago

Issue Description

Now on v3 config is a map, but I have weird behavior with it. If I have this config:

database: {
      stores: {
        teststore: {
          uri: 'mongodb://localhost:27017/test',
          options: {

          }
        },
        storeoverride: {
          uri: 'mongodb://localhost:27017/test2',
          options: {

          }
        },
        notMongoStore: {
          uri: 'postgres://localhost:5432/tests',
          options: {}
        }
      },
      models: {
        defaultStore: 'teststore',
        migrate: 'drop'
      }
    }

If I do: app.config.get('database') => {models:{} , orm: 'mongoose'} app.config.get('database.stores') => {teststore: {},storeoverride: {},notMongoStore: {}} app.config.get('database.models') => {} app.config.get('database.models.defaultStore') => teststore

Doesn't make any sens :( for my use case I want to retrieve all the database object at once and do my configuration with it, I can't as most of the information is not present directly.

Any idea ? advice ? Is there a trick on V3 to make it work ?

Environment

jaumard commented 6 years ago

You can see the problem on my test to migrate this trailpack to v3 https://github.com/trailsjs/trailpack-mongoose/tree/v3 if you run tests you'll see that they don't pass anymore with trails v3

scott-wyatt commented 6 years ago

@jaumard the stores has been abstracted from config/database to config/stores. For trailpack-sequelize v3, I moved models to it's own config and made the pack use those to do model definitions/defaults. I think that it's a pretty good schema, because in way, stores can be used as more then just "databases" and models are not necessarily only related to databases either, so it makes sense to abstract them a bit. Like here: https://github.com/trailsjs/trailpack-sequelize/blob/v3/index.js#L47 and here: https://github.com/trailsjs/trailpack-sequelize/blob/v3/lib/transformer.js. I got this idea for trailpack-knex that @tjwebb did.

jaumard commented 6 years ago

@scott-wyatt right I didn't notice before thanks, but doesn't change the fact that this map config doesn't work as it should logically (or I miss something)

jaumard commented 6 years ago

Should be ok with v3.2.1