Public API for using custom data type parsers

[felixfbecker]

Having specific data types returned as custom types is a common use case. For example, while it makes sense for sequelize to return DECIMALs as strings by default to keep the dependencies small, any application that handles any kind of complex monetary values that need to be manipulated will likely want to use something like bignumber.js to handle those.

const user = await User.findOne();
user.balance instanceof BigNumber // true

await user.update({balance: user.balance.plus('5000000000.10')});

Currently, there are two methods, $stringify (which is private?) and parse. These partly sit on the base data types and then sometimes get overridden by the dialect data-types.

There should be a way to define a custom parser and stringifier for a data type that

• works with all dialects
• does not require to use private methods
• has an easy API (the user should not be concerned about the nature of the data types being static, classes and factory functions).
  support for DataTypes.ARRAY(DataTypes.ENUM('value 1', 'value 2')) in postgres #1498 Proposal

Allow to set two methods on each data type, parse and stringify. parse at the moment is a static method, because it needs to be set in the libraries before a model is defined. stringify() is an instance method and can therefor access this.options. NOTE: It would be nice if parse() could be an instance method aswell for consistency. This may be changed before v4 is stable.

The internal parse and stringify methods will be renamed to _parse and _stringify to make them explicitely private. These may be overridden by dialects. They always first check for the presence of a parse and/or stringify function on the data type that may have been provided by the user and use it to convert the value.

The default implementation of parse() and stringify() always delegate to the dialect-specific data-types with .call()/.apply(). The dialect that should be used when stringifying is given as a dialect option in the options argument that some data types already accept. Data types are not replaced anymore at Model normalization with the dialect specific data types.

A user can choose to fallback to the default behaviour by calling super.stringify().

Sequelize.DECIMAL.parse = value => new BigNumber(value);
Sequelize.DECIMAL.stringify = value => value instanceof BigNumber ? value.toString() : value;

Interface:

stringify?: (value: any, options: { dialect: string}) => string;
parse?: (value: string, options: { dialect: string }) => any;

Questions about the current codebase

• why is $stringify() private, but parse() not? No reason
• why is parse() static, but $stringify() an instance method? Parsers need to be set at connection-level before Model creation
• why is there a second stringify() method? No reason
• why is there no default implementation for parse()? E.g. why are there some data types that don't have this method defined, and how is this handled No reason

ToDo

• Remove ParserStore Overhaul type parsers #6473
• Don't export closures in dialect data types
• Don't patch BaseTypes with types property - just use dialect objects
• Remove DataType.extend (we have inheritance for that and will use delegation anyway)
• Remove DataType.key (we have DataType.name and DataType.prototype.toSql() for that)
• Introduce dialects/abstract/data-types.js (dialects need a base implementation that they can fallback to even when the user overrides parsers/stringifiers)
• Use method delegation

[janmeier]

• why is $stringify() private, but parse() not?
  A brainfart by me most likely - I didn't consider that datatypes can simply override the method. There's no reason for it to be private, especially not when we want users to extend it
• why is parse() static, but $stringify() an instance method?
  Again, can't really remember..
• why is there no default implementation for parse()? E.g. why are there some data types that don't have this method defined, and how is this handled
  Seems that its just assumed that datatypes always have a parse method - Though thats not always the case. It would make sense to have a default implementation for parse as well

Your proposal sounds good to me - I would really like to get the API finalized and documented :)

[felixfbecker]

Great. One thing that needs to be taken into account though is #6270. For example, if a user overrides the parser/stringifier to use BigNumber for DECIMALs, the clone needs to be done with new BigNumber(value) and the comparison with value.equals(previousValue). I think in addition to parse() and stringify(), we need clone() and isEqual().

[janmeier]

Probably a good idea, yea :)

[felixfbecker]

Also, stringify and parse should probably be instance methods because the may need to access parameters like binary for STRING etc.

[felixfbecker]

@janmeier what is the purpose of parserStore? I dont fully understand how the dialect specific data types override the base ones even though you use the base in your models...

[janmeier]

what is the purpose of parserStore?
Postgres handles parser on the library level, https://github.com/sequelize/sequelize/blob/master/lib/dialects/postgres/connection-manager.js#L41, which means parsers are shared across all sequelize instances.

Whereas mysql wants the typeparser on connection level https://github.com/sequelize/sequelize/blob/master/lib/dialects/postgres/connection-manager.js#L41.

To maintain the same behaviour across both (parsers are shared across all sequelize instances) I wanted to store the parsers outside of the connection mananger etc. for mysql. But using a map thats initialized outside of the class works fine as well :) https://github.com/sequelize/sequelize/blob/master/lib/dialects/mysql/connection-manager.js#L9

I dont fully understand how the dialect specific data types override the base ones even though you use the base in your models...
Types are converted to the dialect specific ones in normalizeDatatype

[felixfbecker]

It turns out the implementation is really tricky. One thing that I would like to ensure is that users can save a reference to the default parser, because it can be quite complex. For example, if a user wants to override the ARRAY parser and stringifier to return a Set instead of an Array, he would like to convert the Set to an Array and then call the default stringifier. That would not work if the default methods were made private. But if you just call them stringify and parse, a user-provided function would get overridden by the dialect specific data type.

I feel like the whole inheritance of the dialect-specific data types is a pattern that imposes problems, and maybe we should move away from it. Maybe just provide the dialect-specific ones in a dialect object on the base data type? The object could be exported from the dialects types folder and required by the base data types, which is prettier than the exported closure. That way we don't have to recreate them in normalizeDatatype. But I dont know how we would do the dialect-specific constructor behaviour that way, as some dialects warn about unsupported options. They could probably only warn at parse/stringify. Which is okay, because if we want to support connection-independent models one day we won't have the dialect information available at definition time anyway. The warnings could be abstracted in a method checkDialectSupport or something like that. But the methods of course need to access the data type options, and I don't know how to solve that. I think we really need to rethink the data type / dialect inheritance design.

Would really like to hear some more opinions @sequelize

[felixfbecker]

To sum up the architectural problems here:

• the dialects need to override and share behaviour of the base classes (which currently means they inherit from it)
• we need to allow the user to override something on the base classes wihout fiddling with the dialects(which means they should sit on top of the prototype chain, but they don't)

The solution would be to favor composition over inheritance, but I don't know exactly how. Maybe we can also leverage some JS features like Object.create() or Function.prototype.call().

[felixfbecker]

Btw, node-postgres now supports connection-specific type parsers by using Client.prototype.setTypeParser(). We should use that.

[janmeier]

Definitely - Then we can get rid of the parser store thing, and you can have different parsers for different instances of sequelize

[felixfbecker]

https://github.com/sequelize/sequelize/blob/master/lib/sql-string.js#L39

This is a bit worrying, because stringify() can access options under this.options.

[janmeier]

Hmm yeah - That case handles dates inserted into raw queries (sequelize.query(sql, { replacements }) - But we should definitely ensure that this is defihed

[shtse8]

any updates of this issues? I want to extend some datatypes properly.

[kingjerod]

Currently trying to figure out how to deal with DECIMAL being strings in Sequelize v4, really need something like this.

[damianobarbati]

I'm using raw queries and I see timestamp fields converted to javascript Date object.
How can I configure sequelize to use another object (i.e. Luxon) for a given datatype?
I have no model definition, just raw queries!