Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to specify entities glob instead of entitiesDir glob #605

Closed
ghempton opened this issue Jun 9, 2020 · 17 comments
Closed

Ability to specify entities glob instead of entitiesDir glob #605

ghempton opened this issue Jun 9, 2020 · 17 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ghempton
Copy link

ghempton commented Jun 9, 2020

Is your feature request related to a problem? Please describe.

I am trying to introduce mikro into an existing nestjs project. The entities are all located in separate folders, e.g. 'posts/post.entity.ts, comments/comment.entity.ts. There is no parent entities dir and instead I would like to use something like 'dist/**/*.entity{.ts,.js}'.

Describe the solution you'd like

Allow the entities configuration option to allow for glob paths in addition to the current array of classes. Alternatively, support an entitiesGlob option.

Describe alternatives you've considered

Specifying the entire project as the entitiesDir glob. This seems suboptimal.

@ghempton ghempton added the enhancement New feature or request label Jun 9, 2020
@B4nan
Copy link
Member

B4nan commented Jun 9, 2020

You can do that yourself outside of the ORM, something like this:

import globby from 'globby';

const orm = await MikroORM.init({
  entities: await (globby('./dist/**/*.entity{.ts,.js}')).map(e => require(e)),
  // ...
});

@ghempton
Copy link
Author

ghempton commented Jun 9, 2020

After playing around with that approach for a while, I get an error complaining that there are only abstract entities. I think the issue is that the named export needs to be extracted (e.g. extracting export class Post. It looks like this logic is already in MetadataDiscovery but would need to be utilized somehow. Having this be a configuration option would make this much easier.

@B4nan
Copy link
Member

B4nan commented Jun 9, 2020

The whole metadata discovery process happens for this approach too, there is no real difference.

This validation error most probably means that you forgot to use @Entity() decorator.

@ghempton
Copy link
Author

ghempton commented Jun 9, 2020

I definitely am using the @Entity() decorator and it works when I explicitly add [Post] to entities. I think I am just now realizing that mikro-orm only works with its own annotations and not with other libraries. E.g. my class:

@Entity()
@ObjectType()
export class Post {
  @Field()
  @PrimaryKey()
  id: string = uuid();
}

Where the other annotations are part of @nestjs/graphql. Is there any way to make this work?

@B4nan
Copy link
Member

B4nan commented Jun 9, 2020

I dont see a reason why that should not work, there are many users of typegraphql here.

@ghempton
Copy link
Author

ghempton commented Jun 9, 2020

Appreciate the fast responses and for all your work on this library!

When I explicitly add [Post] to entities I get a new error: ValidationError: Multiple property decorators and looking at the code it seems to not like other annotations. Is this not the case and is mostly likely something else I am doing?

@B4nan
Copy link
Member

B4nan commented Jun 9, 2020

That error means you have multiple decorators from MikroORM on the same property, like @Property() @ManyToOne(). It has nothing to do with other libraries :)

Some users had issues with this error due to watch modes, as at one time there could exist 2 files that exported the same entity. That particular issue is resolved in v4 where the actual path and filename of the entity also matters.

@ghempton
Copy link
Author

ghempton commented Jun 9, 2020

Great, glad to hear it should work and it is on my end :).

I definitely do not have any other annotations from mikroorm on the property. I wonder if somehow my entity is getting processed multiple times or something (maybe related to the nestjs-mikro-orm) package. I will keep digging...

@ghempton
Copy link
Author

ghempton commented Jun 9, 2020

Looks like this is related to having typeorm in the same project at the same time. Looks like typeorm is reloading the class definition somehow... ugh.

@ghempton
Copy link
Author

ghempton commented Jun 9, 2020

For anyone else that stumbles across this, was able to get this work by changing the order of imports so Mikro comes before TypeOrm:

MikroOrmModule.forRoot(mikroconfig),
TypeOrmModule.forRoot(ormconfig),

instead of

TypeOrmModule.forRoot(ormconfig),
MikroOrmModule.forRoot(mikroconfig),

@B4nan
Copy link
Member

B4nan commented Jun 9, 2020

Interesting. I guess I could make that validation more precise, now it just checks the global metadata storage wheather something is already defined for that property or not - if that thing is exactly the same type as the parameter, we can skip that validation.

Its purpose is really just to avoid multiple different property decorators.

@B4nan B4nan added this to the 4.0 milestone Jun 11, 2020
@B4nan B4nan mentioned this issue Jun 11, 2020
46 tasks
@B4nan B4nan added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 11, 2020
@B4nan B4nan self-assigned this Jun 21, 2020
@B4nan B4nan removed good first issue Good for newcomers help wanted Extra attention is needed labels Jun 21, 2020
@chrisbrantley
Copy link

I am suddenly getting the ValidationError: Multiple property decorators used on X property today. I can't figure out what caused it. Especially since it's on an entity that I haven't touched in a long time.

I am also using TypeGraphql in my project but Mikro and TypeGraphql have coexisted just fine.

@chrisbrantley
Copy link

chrisbrantley commented Jun 21, 2020

I found my offending code that is triggering this error but it doesn't make any sense to me.

In a resolver function I get a Repository instance like:

const domainRepository = ctx.em.getRepository(Domain);

If I use the entity name as a string (rather than the class itself) the error goes away.

The other surprising thing is that this block of code is not even executed at startup, yet somehow it's triggering this error at startup.

Any idea why just getting a repository would cause this?

B4nan added a commit that referenced this issue Jun 21, 2020
`entitiesDirs` and `entitiesDirsTs` were removed in favour of `entities` and `entitiesTs`,
`entities` will be used as a default for `entitiesTs` (that is used when we detect `ts-node`).

`entities` can now contain mixture of paths to directories, globs pointing to entities,
or references to the entities or instances of `EntitySchema`. Negative globs are also
supported.

This basically means that all you need to change is renaming `entitiesDirs` to `entities`.

```typescript
MikroORM.init({
  entities: ['dist/**/entities', 'dist/**/*.entity.js', FooBar, FooBaz],
  entitiesTs: ['src/**/entities', 'src/**/*.entity.ts', FooBar, FooBaz],
});
```

BREAKING CHANGE:
`entitiesDirs` and `entitiesDirsTs` are removed in favour of `entities` and `entitiesTs`.

Closes #605
@B4nan
Copy link
Member

B4nan commented Jun 21, 2020

Hard to say, but keep in mind this is metadata validation error - it is thrown during ORM initialisation, it is definitely not caused by you getting a repository. As said, this should be hopefully fixed in v4, so you could give that a try. Will be releasing new alpha soon.

B4nan added a commit that referenced this issue Jun 21, 2020
`entitiesDirs` and `entitiesDirsTs` were removed in favour of `entities` and `entitiesTs`,
`entities` will be used as a default for `entitiesTs` (that is used when we detect `ts-node`).

`entities` can now contain mixture of paths to directories, globs pointing to entities,
or references to the entities or instances of `EntitySchema`. Negative globs are also
supported.

This basically means that all you need to change is renaming `entitiesDirs` to `entities`.

```typescript
MikroORM.init({
  entities: ['dist/**/entities', 'dist/**/*.entity.js', FooBar, FooBaz],
  entitiesTs: ['src/**/entities', 'src/**/*.entity.ts', FooBar, FooBaz],
});
```

BREAKING CHANGE:
`entitiesDirs` and `entitiesDirsTs` are removed in favour of `entities` and `entitiesTs`.

Closes #605
@chrisbrantley
Copy link

That was my understanding, which was why I was so surprised that a line in a function that is only called at runtime was causing the problem.

For now I’m just catching this specific error at startup and ignoring it. Obviously a hack.

I’ll try the latest 4.0 alpha and see how it behaves.

@chrisbrantley
Copy link

4.0 alpha-3 solves my problem

B4nan added a commit that referenced this issue Jun 22, 2020
`entitiesDirs` and `entitiesDirsTs` were removed in favour of `entities` and `entitiesTs`,
`entities` will be used as a default for `entitiesTs` (that is used when we detect `ts-node`).

`entities` can now contain mixture of paths to directories, globs pointing to entities,
or references to the entities or instances of `EntitySchema`. Negative globs are also
supported.

This basically means that all you need to change is renaming `entitiesDirs` to `entities`.

```typescript
MikroORM.init({
  entities: ['dist/**/entities', 'dist/**/*.entity.js', FooBar, FooBaz],
  entitiesTs: ['src/**/entities', 'src/**/*.entity.ts', FooBar, FooBaz],
});
```

BREAKING CHANGE:
`entitiesDirs` and `entitiesDirsTs` are removed in favour of `entities` and `entitiesTs`.

Closes #605
B4nan added a commit that referenced this issue Jun 22, 2020
`entitiesDirs` and `entitiesDirsTs` were removed in favour of `entities` and `entitiesTs`,
`entities` will be used as a default for `entitiesTs` (that is used when we detect `ts-node`).

`entities` can now contain mixture of paths to directories, globs pointing to entities,
or references to the entities or instances of `EntitySchema`. Negative globs are also
supported.

This basically means that all you need to change is renaming `entitiesDirs` to `entities`.

```typescript
MikroORM.init({
  entities: ['dist/**/entities', 'dist/**/*.entity.js', FooBar, FooBaz],
  entitiesTs: ['src/**/entities', 'src/**/*.entity.ts', FooBar, FooBaz],
});
```

BREAKING CHANGE:
`entitiesDirs` and `entitiesDirsTs` are removed in favour of `entities` and `entitiesTs`.

Closes #605
@B4nan
Copy link
Member

B4nan commented Jun 22, 2020

Closing as implemented in v4 via #618, will be part of 4.0.0-alpha.4.

@B4nan B4nan closed this as completed Jun 22, 2020
B4nan added a commit that referenced this issue Aug 2, 2020
`entitiesDirs` and `entitiesDirsTs` were removed in favour of `entities` and `entitiesTs`,
`entities` will be used as a default for `entitiesTs` (that is used when we detect `ts-node`).

`entities` can now contain mixture of paths to directories, globs pointing to entities,
or references to the entities or instances of `EntitySchema`. Negative globs are also
supported.

This basically means that all you need to change is renaming `entitiesDirs` to `entities`.

```typescript
MikroORM.init({
  entities: ['dist/**/entities', 'dist/**/*.entity.js', FooBar, FooBaz],
  entitiesTs: ['src/**/entities', 'src/**/*.entity.ts', FooBar, FooBaz],
});
```

BREAKING CHANGE:
`entitiesDirs` and `entitiesDirsTs` are removed in favour of `entities` and `entitiesTs`.

Closes #605
B4nan added a commit that referenced this issue Aug 9, 2020
`entitiesDirs` and `entitiesDirsTs` were removed in favour of `entities` and `entitiesTs`,
`entities` will be used as a default for `entitiesTs` (that is used when we detect `ts-node`).

`entities` can now contain mixture of paths to directories, globs pointing to entities,
or references to the entities or instances of `EntitySchema`. Negative globs are also
supported.

This basically means that all you need to change is renaming `entitiesDirs` to `entities`.

```typescript
MikroORM.init({
  entities: ['dist/**/entities', 'dist/**/*.entity.js', FooBar, FooBaz],
  entitiesTs: ['src/**/entities', 'src/**/*.entity.ts', FooBar, FooBaz],
});
```

BREAKING CHANGE:
`entitiesDirs` and `entitiesDirsTs` are removed in favour of `entities` and `entitiesTs`.

Closes #605
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants