refactor(core): Replace typedi with our custom DI system (no-changelog) #12389
Conversation
n8n-assistant
bot
added
core
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
| /** Clears all instantiated instances from the container while preserving type registrations */ | ||
| reset(): void { |
There was a problem hiding this comment.
Would it be more accurate to rename as clearInstanceCache? factory remains.
There was a problem hiding this comment.
I think the jsdoc should describe the details. The name reset is fine IMO.
There was a problem hiding this comment.
while preserving type registrations → I'm not clear on what's a type registration. If it means the factory let's use that?
There was a problem hiding this comment.
More ideas to test if you like
- Circular dependencies
- Ensure
getafterresetcreates a new instance - Resolution stack gets cleared after an error
There was a problem hiding this comment.
I did add tests for circular dependencies, but had to remove them because circular dependency between 2 classes in the same file causes one of them to be undefined, leading to compile-time, and runtime unhandled error.
in part 2, I'll add more tests after I add the testing related stuff.
n8n
|
||||||||||||||||||||||||||||
| Project |
n8n
|
| Branch Review |
n8n-di
|
| Run status |
|
| Run duration | 04m 49s |
| Commit |
|
| Committer | Iván Ovejero |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
484
|
| View all changes introduced in this branch ↗︎ | |
|
✅ All Cypress E2E specs passed |
Summary
This PR replaces
typediwith a DI system that we can maintain and have full control over.Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)