molszanski / iti Goto Github PK

A useful feature is to be able to dispose of resources from the container.

E.g. disconnect from the databases and so on.

See how Awilix did it.

Problem Description

When using iti in a TypeScript project with "moduleResolution": "NodeNext" in your tsconfig.json, type information for iti cannot be located.

This line:

import { createContainer } from 'iti'

Produces the error:

Could not find a declaration file for module 'iti'. '<project path>/node_modules/iti/dist/iti.modern.js'
implicitly has an 'any' type.
  Try `npm i --save-dev @types/iti` if it exists or add a new declaration (.d.ts) file
  containing `declare module 'iti';`

The project will still compile and run with tsc; you just don't have type information for iti for static checking, linting, etc.

Cause

There are two causes:

1. The exports map in iti's package.json contains an exports map, but the types key is not defined for each export. See this TypeScript issue for a complete explanation. When exports is defined, top-level types or typings keys are ignored.

2. The export ... from statements in iti/src/index.ts need to use an explicit .js extension. Without this, import { createContainer } from 'iti' will nominally succeed, but the type of createContainer will just be import, and the static analysis gets very confused.

Proposed fix

1. The first issue is simple: just add "types": "./dist/src/index.d.ts" to the exports map in package.json.

2. For the second issue, I'll be honest: I'm fairly new to JS/TS. My understanding is that explicit .js extensions are required for import/export statements when using ESM ("type": "module" in package.json). So I'm not sure how iti is getting away with "bare" imports in the first place. But in any case, adding extensions only to the export statements in src/index.ts is enough to satisfy the consumer's linter and should be backwards-compatible with CJS. With that said, two of the iti tests (dispose....) try to import { createContainer } from '../src', which needs to be ../src/iti after this change. The other tests already import this way.

I have a fork with these proposed changes made to iti and iti-react. I can make a PR if you'd like.

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

Considering the following code.

const a = await root.getContainerSet(["userManual", "oven"])
a.oven // There is no error, oven exists, the type in compile-time is the same that the type in run-time

const b = await root.getContainerSet(() => ["userManual"])
b.oven // There is no error, BUT oven DOES NOT exist, the type in compile-time is NOT the same that the type in run-time

It is a problem in this example:

const kitchenContainer = async ({ oven, userManual }) => {
  await oven.preheat()
  return {
    kitchen: new Kitchen(oven, userManual),
  }
}

const node = root.add((ctx, node) => ({
  kitchen: async () =>
    kitchenContainer(await node.getContainerSet(["userManual"])),
}))

We are requiring from kitchenContainer two dependencies: over and userManual. But we are only providing userManual. This will cause an error at run-time.

Describe the solution you'd like

The returned type of getContainerSet should be automatically inferred from what we pass in parameter.

Solution

(tested and it works)

Edit these lines

 public async getContainerSet<T extends keyof Context>( 
   tokensOrCb: Pick<KeysOrCb<Context>, T>, 
 ) { 

(This solution may be applicable to other functions like subscribeToContainerSet and _extractTokens too)

Note: We can improve the type printing (on hovering on VSCode for example), by wrapping the Pick<...> into the Prettify helper.

type Prettify<T> = {
  [K in keyof T]: T[K];
} & {};

With Prettify	Without Prettify

Is your feature request related to a problem? Please describe.
Adding a lot of add(..) slows down Intellisense to the point that I have to restart the TS server every now and then.

Describe the solution you'd like
I was wondering whether it would be feasible to provide a version of iti that has less features but provides faster type inference. I haven't dug into the code yet, just wondering whether something like that is out of the question.

What would be an ideal API for your use case?
I just need add(), upsert() and items(). No async injection or any of that disposal API.

I need to update the package to support both ESM and commonjs better.
Work in progress

If you add too much dependencies it gives "Type instantiation is excessively deep and possibly infinite"
When I added last dependency which just and empty object (to avoid circular dependency case) it gives ts error.

https://stackblitz.com/edit/typescript-s5bnzn?file=index.ts

Hi again,

From what I've read, using the delete operator is very slow.
https://stackoverflow.com/questions/13982306/in-javascript-what-is-more-efficient-deleting-an-element-or-setting-it-to-undef.

Would you ever consider another alternative, like setting deleted properties to undefined?
In addition, is there a reason to not use a Map instead of an object?

Should be subscribeToContainer I think.

Same for subscribeToContinerSet.

Class-based / object dependencies could implement event hooks.

For example

class Logger implements Init {
     init() {
       this.info("logger started");
     }
}

or

{ init : () => this.info("logger started") }

There could be various hooks on dependencies that would be called when an iti even occurs.
Describe the solution you'd like
A clear and concise description of what you want to happen.
Naive solution
For any / all events, loop through dependencies, calling corresponding function.

//init event occurs
for(dependency of Object.values(cache)) {
   dependency?.init?.()
}