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

TypeError: Cannot read property getInstanceProperties' of undefined #114

Closed
gustavopch opened this issue Sep 9, 2019 · 7 comments
Closed
Assignees
Milestone

Comments

@gustavopch
Copy link

Try to create entities in files with same name but in different folders, like:

  • category/entities.ts
  • product/entities.ts
  • user/entities.ts

and you'll get the following error when initializing:

UnhandledPromiseRejectionWarning: TypeError: Cannot read property getInstanceProperties' of undefined

Investigating a bit, I found that TypeScriptMetadataProvider creates a map of source files keyed by the filename, causing some kind of conflict for the above mentioned case where multiple files have the same name but are on different folders.

@gustavopch
Copy link
Author

A possible solution (https://github.com/mikro-orm/mikro-orm/blob/master/lib/metadata/TypeScriptMetadataProvider.ts#L27):

- const file = meta.path.match(/\/[^\/]+$/)[0].replace(/\.js$/, '.ts');
+ const file = meta.path.split(process.cwd())[1].replace(/\.js$/, '.ts');

That way file would be the path relative to the project root and keys wouldn't conflict. Not sure about side-effects.

@B4nan
Copy link
Member

B4nan commented Sep 9, 2019

Currently entity name has to be unique, but your approach is about having multiple entities in the entities.ts file, right?

@gustavopch
Copy link
Author

@B4nan Yes. I'm structuring my project in modules, so that product/entitites.ts would have all product-related entities, user/entities.ts would have all user-related entities etc.

@B4nan
Copy link
Member

B4nan commented Sep 9, 2019

I don't think that is currently supported, it's definitely more complicated than your proposed change. ORM needs to be able to tell the entity name based on the file name and vice versa so it can find the TS source file for type sniffing via reflection.

I would suggest to have each entity in dedicated file and then re-export them in your entities.ts file (generally one would probably call it index.ts). That should be compatible with what you want (you will have single access point for all entities from given module, while every entity will be defined in its own file.

BTW you could also try to specify all types manually via type and entity decorator options. That way ts-morph won't be even used for reflection (as no reflection will be needed).

@gustavopch
Copy link
Author

I don't think that is currently supported, it's definitely more complicated than your proposed change. ORM needs to be able to tell the entity name based on the file name and vice versa so it can find the TS source file for type sniffing via reflection.

I would suggest to have each entity in dedicated file and then re-export them in your entities.ts file (generally one would probably call it index.ts). That should be compatible with what you want (you will have single access point for all entities from given module, while every entity will be defined in its own file.

BTW you could also try to specify all types manually via type and entity decorator options. That way ts-morph won't be even used for reflection (as no reflection will be needed).

In fact, the problem is bigger than what I thought. I'll follow your suggestion to have individual files with barrel export. I'll let you decide whether to close this issue or not as I don't know whether you'd be interested to ship this change with v3.

@B4nan
Copy link
Member

B4nan commented Sep 9, 2019

I will add validation for such case so it will throw a bit more informative error about the requirement for unique file names in v3. Will close this once it's implemented.

@B4nan B4nan self-assigned this Sep 9, 2019
@B4nan B4nan added this to the 3.0 milestone Sep 9, 2019
B4nan added a commit that referenced this issue Sep 10, 2019
Validates that:
- there can be only one entity with given class name
- there are only well named entities (file name vs class name)
- there was at least one discovered entity

Closes #114
B4nan added a commit that referenced this issue Sep 15, 2019
Validates that:
- there can be only one entity with given class name
- there are only well named entities (file name vs class name)
- there was at least one discovered entity

Closes #114
B4nan added a commit that referenced this issue Sep 19, 2019
Validates that:
- there can be only one entity with given class name
- there are only well named entities (file name vs class name)
- there was at least one discovered entity

Closes #114
@B4nan
Copy link
Member

B4nan commented Sep 20, 2019

Closing as resolved. Available for testing via next tag.

@B4nan B4nan closed this as completed Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants