Closed ephys closed 1 year ago
Yes we'd better try to be compatible with that ( if no strong reason for that is not good ), as far as I know all the nest.js might be using that approach already.
The new decorator proposal has reached stage 3, we can consider implementing experimental support if we don't need the metadata part of it.
One of the main issues I have with sequelize-typescript is recursive imports. When I wanted to upgrade NextJs on my project I ran into tons of problems because recursive imports suddenly become errors instead of "just" warnings. They had switched to es6 and some behaviors changed. I wrote a PR (https://github.com/sequelize/sequelize-typescript/pull/1206) to solve this issue and it did, but it was never merged. That's fine. Perhaps it offers some inspiration. My point is: these decorators have a tendency to promote recursive imports and those can create problems.
That's a strange decision by next, recursive imports should work as long as the export is not used during the initial module execution
I don't see why we couldn't include this when merging with sequelize-typescript. Sounds like a good change :)
Everything described in here is implemented now
This is a long one, bear with me.
I'd like to bring built-in support for decorator based model declaration in Sequelize. It's one of the main arguments I've seen in favor of using TypeORM over the years and I think it would help reduce the boilerplate of creating a new model.
I am not going to lie, a lot of this is very inspired by
sequelize-typescript
, with some differences.Prior art
sequelize-typescript
sequelize-decorators
sequelizejs-decorators
Foreword: Legacy Decorators & Stage 2 Decorators
Both the TypeScript & Babel communities have been using Decorators for years. These decorators follow the old decorator spec. A new spec has been in the works for a few years now. It is unclear when stable decorators will actually land in ECMAScript.
As such, I propose to do the initial implementation using Legacy Decorators but expose them through
sequelize/decorators-legacy
. Once decorators become stage 4, we can do a parallel implementation that is directly exposed in the rootsequelize
import, and deprecate/decorators-legacy
.API Design
Model registration
One of the first parts of the design would be to provide a way to register a model that has been decorated. A basic building block that's decorator-implementation agnostic. Both for existing third-party packages and a future stage-4 decorators implementation.
I would expose two methods:
registerModelAttributeOptions
®isterModelOptions
:We then also need a way to register the model to Sequelize. I see a few options:
Model.init
:Sequelize#define
Sequelize#registerModels
models
parameter to the Sequelize constructor.I'd opt for overloading.
Automatic model registration
Similarly to what TypeORM & SequelizeTypescript are doing, we could add an async method
Sequelize#importModels(glob)
that loads files matching the glob (using ESM dynamic import), and register any export that extends model and isn't tagged asabstract
(see Model Inheritance).Model Inheritance
this would resolve https://github.com/sequelize/sequelize/issues/1243
A big benefit of decorator-based definition is that it becomes possible to inherit definitions (both model options & attributes):
Model Options Decorator:
@ModelOptions
Note: Name is already taken by typing. Alternative names:
@Model
(already taken),@Table
,@Entity
, etc...The simplest design for a Model Option decorator would be one that simply accepts the model options:
Model Attribute Decorator
We have two choices here: A simple
Attribute
decorator which acceptsModelAttributeColumnOptions
, or a decorator per-option like with sequelize-typescript.This is the design I came up with, critics and counter-proposals welcome:
Model Association Decorator
This part depends on RFC sequelize/sequelize#14302 and is described over there but is basically: a decorator per association type:
emitDecoratorMetadata
I would vote against guessing which
DataType
to use based on decorator metadata. Having defaults forstring
,number
, etc.. will lead to users accidentally using the Column Type as they're not strict enough.other elements to consider