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 properties of null (reading 'constructor') #1664

Closed
2 of 4 tasks
c100k opened this issue Dec 3, 2024 · 3 comments
Closed
2 of 4 tasks

TypeError: Cannot read properties of null (reading 'constructor') #1664

c100k opened this issue Dec 3, 2024 · 3 comments
Assignees
Labels

Comments

@c100k
Copy link

c100k commented Dec 3, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

When executing tests via vitest, I have the following failure (that is not present in 6.0.3):

TypeError: Cannot read properties of null (reading 'constructor')
  getBaseClassDependencyCount node_modules/src/planning/reflection_utils.ts:22:32
  forEach node_modules/src/planning/planner.ts:268:50
  _createSubRequests node_modules/src/planning/planner.ts:239:18
  forEach node_modules/src/planning/planner.ts:282:9
  forEach node_modules/src/planning/planner.ts:281:20
  _createSubRequests node_modules/src/planning/planner.ts:239:18
  node_modules/src/planning/planner.ts:282:9

Steps to reproduce

No response

Expected behavior

Ideally, the code shouldn't fail like this. Indeed, there is no problem with 6.0.3. From 6.1.0 and subsequent versions it fails. These changes introduced in 6.1.0 in https://github.com/inversify/InversifyJS/blob/master/src/planning/reflection_utils.ts#L20 seem very hacky with the as and the eslint rules disabled.

On the other side, when the error occurs, the stacktrace should give the correct paths. Right now, it gives node_modules/src/planning/reflection_utils.ts which is not the correct path. I think there is a problem with the sourcemap generation which goes one directory too far because of ../../../.

Additionnally, I'm not sure it's a good idea to ship esm already bundled. Indeed, it makes it almost impossible to debug internally in order to find out the underlying issue of such an error.

Possible solution

I suspect there is an issue with constructor arguments that are not "injected". But I'm still digging to understand why it works on 6.0.3 and why it has stopped working with 6.1.0 and subsequent versions.

When I add console.log in the bundled ESM code, I'm getting this:

{ metadataReader: E {}, func: [class Getter], t: 'function' }
{ metadataReader: E {}, func: [Function: Object], t: 'function' }

Indeed, this function seem to be able to receive something else than a Newable so it must make additional checks and avoid at all prices "casting" the type.

Package version

6.1.5

Node.js version

22

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Stack trace

No response

Other

No response

@notaphplover
Copy link
Member

Hey @c100k, thank you for your report. Please submit a minimum recreation code via code sandbox or any other platform. A reproduction repository with the minimum reproduction code is a valid choice as well.

@notaphplover
Copy link
Member

notaphplover commented Dec 3, 2024

I see the issue, on my way to patch it.

Additionnally, I'm not sure it's a good idea to ship esm already bundled. Indeed, it makes it almost impossible to debug internally in order to find out the underlying issue of such an error.

I wish there was a better way to provide an ESM build compatible with NodeJS, browsers and bundlers in any typescript config. Not my cup of tea as well, but I don't see a better approach.

On the other side, when the error occurs, the stacktrace should give the correct paths. Right now, it gives node_modules/src/planning/reflection_utils.ts which is not the correct path. I think there is a problem with the sourcemap generation which goes one directory too far because of ../../../.

I see, indeed, esm source maps have the wrong parent directory 🤔, it seems rollup is not properly generating them. I'll patch that as well

@notaphplover notaphplover moved this from Backlog to Ready in Inversify maintenance Dec 3, 2024
@notaphplover notaphplover moved this from Ready to In progress in Inversify maintenance Dec 3, 2024
@notaphplover notaphplover self-assigned this Dec 3, 2024
@notaphplover notaphplover moved this from In progress to In review in Inversify maintenance Dec 3, 2024
@notaphplover
Copy link
Member

Fixed in #1665, released in [email protected]. Closing the issue now. If the issue persist, just submit a recreation code and I'll reopen it.

@github-project-automation github-project-automation bot moved this from In review to Done in Inversify maintenance Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

No branches or pull requests

2 participants