Sequelize-typescript: Model loading, ignore declaration files

[snewell92]

When passing a directory to the modelPaths option in the constructor, it would be nice if sequelize-typescript only looked for .js files.

I see two instances where people could run into this:

1. mocha-typescript requires my main application to be compiled with the "declarations": true option so that it can consume the application's types to (if the test suite and app are two different typescript apps). Unfortunately, this means in my output directory not only will typescript emit js files but also .d.ts. Anyone else using declarations will run into this.

2. Any source maps map file alongside the models will also junk up the output directory.

This can be easily worked around with a little but of code with fs and some filtering, but maybe this extra nicety could be baked into sequelize-typescript? I'm going to be writing up a PR, since I'll be using the same logic in my own code if this doesn't make it into the repo. Feel free to either close this issue if you don't want it baked in, or assign it to me if you do.

Thanks for reading! <3<3

[RobinBuschmann]

Hey @snewell92, on which version did you noticed this? I've released a new version(0.3.3) several hours ago, in which I changed this behavior. With 0.3.2 it used .js files only, but now also considers .ts files too. This is because of what is described here: #22

Unfortunately I didn't think about d.ts files. So, yes, this need to be fixed, but loading .ts files need to be further supported.

Probably it would make sense to add an exclude option or support globs instead of a directory only in modelPaths. What do you think?

[snewell92]

Ah yes I saw that patch earlier and should've connected the dots.

I also upgraded to 0.3.3 earlier today.

Let's discuss this, because ideally we would allow end users total freedom in their project structure. Source maps, definition files, a .js output or .ts to be dynamically transpiled by ts-node - possibly even an index file that is a single source of exporting models.

I like your file glob idea, but I also like the excludes option.

The file glob elegantly takes care of .ts/.js/.map/.d.ts file inclusion, and solves it for future file types that might encroach our TS Sequelize projects. We could even keep current behavior first checking if the string is a directory, I recently wrote some code that figures that out: fs.lstatSync(path.join(dirPath, file)).isDirectory().

The modelExcludes option seems easy, but I'm not really sure.

I'm a bit confused as to how the directory gets resolved into the files/models. I was looking in the Sequelize.ts code in the v3 and v4 folders, and both declare an addModels function but I can't find that function in the upstream plain sequelize library. Or is this code in the BaseSequelize.ts file around line 67ish? So this code would have to change?

BaseSequelize.ts

addModels(arg: Array<typeof Model|string>): void {

 const models = getModels(arg);

 this.defineModels(models);
 models.forEach(model => model.isInitialized = true);
 this.associateModels(models);
 resolveScopes(models);
 models.forEach(model => this._[model.name] = model);
}

I'd be willing to take a crack at either or both of these methods (supporting file glob / support excludes option) 😸 I actually initially had an index.ts file with my models, and I had to move all my model to their own folder. With the excludes option I wouldn't have to.

[RobinBuschmann]

OK, let's support both options then :)

The v3/v4 folder structure together with BaseSequelize seems to be a little bit confusing, I know. But it is necessary to support version 3 and version 4 at the same time. Sequelize 4.0.0 provides es6 classes, Sequelize >= 3 the constructor-pattern. Since es6 classes cannot be extended with constructor-pattern "classes", the models in v4 are transpiled with target "es6" and everything else with "es5". That's why BaseSequelize is not extended by v3/Sequelize.ts and v4/Sequelize.ts directly.

The models get resolved in services/models.ts -> getModels.

We should use glob or minimatch for our glob implementation.

Should the modelPaths option be changed to support globs or should we add new options like include/exclude? If so, will modelPaths and include/exclude work together or only the one or the other?
I think modelPaths should expect a path as the name of the option says. So from my opinion, we should use an additional option like include. Furthermore I see no problems if modelPath and exclude will used together and include with exclude should work as well. modelPath with include should be forbidden.

Nevertheless I would like to exclude d.ts files in modelPath by default, as a quick fix.

[snewell92]

tldr; I like this. I would like the includes option to be an array of globs/directories/paths that gets concatenated with the modelPaths array. Excludes option can be an array of strings that are patterns/file names. The readme/documentation can go unchanged until includes/excludes gets implemented. modelPaths, as far the end user is concerned, can be for directories, includes for glob patterns. The code won't care since it handles both the same. First, I'll add the .d.ts exclusion by default with a PR to close this issue.

v3/v4 does makes sense for supporting both versions, I just didn't know the structure of the project very well, still digging into it, thanks for explaining where getModels is and how this project supports the two version (Which is very nice by the way! Great job on that :+1: ).

As far as how globbing and include/exclude would work together, I think we should keep the same behaviour and implementation - we can add file globbing as an opt in choice. It would be easy to detect if modelPaths is a directory or not, and if so just do what we do now. (With includes option concatenated, also see if the current string is a file or not) Otherwise try it as a glob. If that fails throw an error because those are the only ~two~three options we support.

As far as how include/exclude + modelPaths will work all together, I like your thoughts about taking care with how these options would interact.

....

After thinking about it a little more, what if the modelPaths is a directory that specifies a main model folder, looks at excludes option to exclude an index file, and also there is one rogue model in another directory, for whatever reason. Or perhaps there are two folders, one for entities, another for associations or 'higher-order' models. So it doesn't make sense to use includes to include a file that is in the directory, as we would have already pulled it in, but if it specifies one rogue file somewhere that makes sense. The second folder we already can pull in because we expect an array of directories.

Thus, I think includes/excludes would work the same way with both options, modelPaths as directory or glob. And we can do the discrimination within the reduce loop so we can mix directories with glob patterns.

Your point about the name of the option (modelPaths should be a path, since that's the name of the option) is a good one though. With my conclusions modelPaths is either a path, or a glob - which is not only a path. What we can do is add the includes option as an array of string that are either paths/globs, and just join the two arrays together and iterate through them. The includes option is a little broader in that I could see people passing specific file names, but I know how to discriminate between directory/file. It would be easier to implement this way, and I see no real reason to enforce that modelPaths are only paths. The readme/documentation can stay the same about modelPaths (accepts a path) and then add a section about the includes and excludes portion.

Example configs

{
 modelPaths: [ path.join(__dirname, 'models') ],
 includes: [ './src/models/associations/*.js' ],
 excludes: [ 'index.js' ]
}

{
 includes: [ './src/models/**/*.js ], // grab all js files
 excludes: [ 'index.js' ] // remove index.js files (app and tests use this to import models)
}

To recap (to demonstrate that I understand, so I don't go off coding the wrong thing):

• First
  Exclude .d.ts file by default in getModels with the current implementation (probably its own PR, at the very least definitely its own commit) - quick fix for right now.
• Second
  
  
  
  • Add the includes option, just concatenate this array to modelPaths in BaseSequelize.init. Need some code to check that both exist (default to empty arrays).
  
  
  • Implement discerning whether modelPaths is a directory. If so, do what we do now.
  
  
  • If not a directory, assume a glob, use glob.
  
  
  • After grabbing initial slew of files, apply the excludes/include option to filter or add in other files. Then we can require them result in.
  
  

How about I get a PR to close this issues about .d.ts file right now, since now I see how those files are filtered, and then I open a separate linked issue about adding includes/excludes, and I'll open a separate PR where I start working on the aforementioned second task.

Let me know if you disagree with the concatenation of modelPaths and includes array. I think this is a good compromise, docs can keep the semantics of modelPaths while the code becomes easier to wrangle.

[RobinBuschmann]

Hey @snewell92, thanks for your input.

Regarding include/exclude options: I'm a little bit confused now :O You wrote

Your point about the name of the option (modelPaths should be a path, since that's the name of the option) is a good one though. With my conclusions modelPaths is either a path, or a glob - which is not only a path.

But later on

Implement discerning whether modelPaths is a directory. If so, do what we do now.
If not a directory, assume a glob, use glob.

So you meant to support globs in modelPaths, right? Hm, I'm not sure about it. Why should one use modelPaths and include together? Either could work on its own or did I overseen something?

Regarding your example includes: [ './src/models/associations/*.js' ],: Setting a glob in include without a leading __dirname is probably not enough. So either the glob need to be joined with __dirname or another option, in which a "root" is defined, is necessary. But this would always look like:
{root: __dirname}
Do you know what I mean? But probably there is a lack of understanding on my side.

[snewell92]

Yeah that long comment was confusing. But you are right, I was saying to support globs in modelPaths This is what I've come to:

I still think we should keep the current behavior so people's code doesn't break, so modelsPaths should still be able to be a directory. Now,should it only be a directory? Or can it be a glob too?

If we allow modelPaths to be an arbitrary mix of directories/globs, implementing includes will be simpler, since we can concat both arrays, and in the reduce loop in getModels we have extra code before the directory read (fs.readDirSync I think) where we look at the string we have and test if it is a directory, file, or glob.

If you think it is simpler, we could make the getModels option only a directory, and the includes only files / glob.

In regards to the example: it does need either a root option, or to be fully qualified with __dirname - I was just looking at what glob has in the readme - it looked like they just pass in the glob pattern and maybe internally they do the __dirname join (?). But you are right, better to be explicit with a root option or expect users to pass path.join(__dirname, 'src', 'models', 'associations', '*.js')

[snewell92]

I'm thinking about what you said...

Either could work on its own or did I overseen something?

maybe you're right. Unless we want to be strict, we could simply not use the includes option, and just use modelPaths.

[leogoesger]

What is the status on the exclude? I am trying to use modelPaths to include all my models but index.ts file. Is this possible? Thank you very much for the lib

[RobinBuschmann]

@leogoesger Since globs are supported now, you could do it like so: /**/!(index).ts