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

Type resolution fails when importing iti with "moduleResolution": "NodeNext" #37

Open
tmillican opened this issue Jan 10, 2023 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@tmillican
Copy link

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.

@tmillican tmillican added the bug Something isn't working label Jan 10, 2023
@molszanski molszanski self-assigned this Jan 11, 2023
@molszanski
Copy link
Owner

molszanski commented Jan 11, 2023

Oh great! Thank you @tmillican for opening this up! Will check it and merge into the upstream 🙇‍♂️

@molszanski
Copy link
Owner

molszanski commented Jan 11, 2023

Also, this is a great description, I've learned something new :)

@danielmahon
Copy link

@molszanski thoughts on getting this fix added?

@molszanski
Copy link
Owner

@danielmahon oh!
Yeah, forgot about it! Will release a minor with better cjs/mjs compatibility!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants