Logging option not working in 0.5.0.beta.1

[bilby91]

Hello,

The logging option it doesn't seem to be working correctly. This is the configuration I'm using to instantiate Sequelize.

{ 
      dialect: 'postgres',
      uri: 'postgres://suttna:development@localhost:5432/db_test',
      logging: false,
      modelPaths: [ '/Users/bilby91/Projects/app/bot/src/app/models' ],
      pool: { max: 5, min: 0, idle: 10000 } 
}

With that configuration I'm getting log messages every time an interaction with the db is happening.

[bilby91]

The problem seems to be happening only when using the uri format.

[RobinBuschmann]

@bilby91 Thanks for reporting.

This is because all other options are ignored if uri is part of the object literal:

      (typeof config === "string") ?
        config : // URI string
        BaseSequelize.isISequelizeUriConfig(config) ?
          config.uri : // <= here
          BaseSequelize.prepareConfig(config) // Config object (ISequelizeConfig)

@kukoo1 Can the pure sequelize handle the uri option? If not I think we should remove the possibility to add uri in the object literal. What do you think about it? Do I miss something?

[bilby91]

@RobinBuschmann I'm trying to fix it. Their a overload of the native Sequelize Constructor that expects the uri and the seconds param is the options. The problem is that I'm not sure how to make it work :(

This is not compiling. I thought that the ... could do the trick. Any ideas ?

constructor(config: SequelizeConfig | string) {
    super(
      typeof config === "string" ?
        config :
          BaseSequelize.isISequelizeUriConfig(config) ?
            ...[config.uri, config] : config
    )

    if (typeof config !== "string") {
      this.init(config);
    }
  }

[RobinBuschmann]

@bilby91 Thank you for your ambitions. Yeah, you're right, the spread operator in this case is not supported by typescript. This is why the uri support was so difficult to implement. See the pull request which delivered this feature for more information: #79

[bilby91]

@RobinBuschmann Hey, I have a fix. I'm testing it now.

[bilby91]

@RobinBuschmann This is working. I had to move the defaults for throughMap and _ so that I can call super in different places.

constructor(config: SequelizeConfig | string) {
    if (typeof config === "string" || !BaseSequelize.isISequelizeUriConfig(config)) {
      super(config)
    } else {
      super(config.uri, config)
    }

    this.throughMap = {}
    this._ = {}

    if (typeof config !== "string") {
      this.init(config);
    }
  }

[RobinBuschmann]

@bilby91 But tsc throws: TS2376: A 'super' call must be the first statement in the constructor when a class contains initialized properties or has parameter properties.

[bilby91]

@RobinBuschmann Nop, I also changed this.

  throughMap: {[through: string]: any};
  _: {[modelName: string]: typeof Model};

I had to remove the defaults here because the was generating an implicit this call

[RobinBuschmann]

@bilby91 Ahhh ok.. so the implicit this statements causing the A 'super' call must be the first.... That makes sense.

[bilby91]

@RobinBuschmann Let's see if the tests pass :)

[kukoo1]

awesome @bilby91 :)
I tried to figure that for a week but no luck haha