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

TypeScript cannot find declaration files with module nodenext #960

Closed
cseas opened this issue Jan 17, 2023 · 15 comments · Fixed by #970
Closed

TypeScript cannot find declaration files with module nodenext #960

cseas opened this issue Jan 17, 2023 · 15 comments · Fixed by #970
Labels

Comments

@cseas
Copy link
Member

cseas commented Jan 17, 2023

Problem

Blade ships with d.ts declaration files. However, since the bundled package.json doesn't contain a "types" field, the consumer project is unable to find these declaration files and as a result ignores the types for components that Blade ships.

VSCode can't find the declaration files either as shown by the below warning:

Screenshot 2023-01-17 at 1 10 50 PM

This means that IntelliSense in the editor doesn't work for types/props. Also, passing wrong props to a component works. For example, the below component compiles without errors.

<Button hello="world">Pay Now</Button>

Solution

The package.json should contain a "types" field that points to the declaration file so the bundler and IDE can find it.

Refer ESM guide for types in exports field: https://www.typescriptlang.org/docs/handbook/esm-node.html

Note that the fallback types field in the above link is required even for ESM bundles right now because proper support for exports field is a part of TypeScript 5 milestone: microsoft/TypeScript#50794 (comment)

@cseas cseas added the bug label Jan 17, 2023
@divyanshu013
Copy link
Contributor

Interesting, I wonder how's it working for other projects right now 🤔

@kamleshchandnani
Copy link
Collaborator

kamleshchandnani commented Jan 17, 2023

from the docs link you mentioned

Also note that if your main declaration file is named index.d.ts and lives at the root of the package (next to index.js) you do not need to mark the types property, though it is advisable to do so.

and it applies to us since our main file is named as index.web.js
Screenshot 2023-01-17 at 7 58 40 PM

Can you link a possible codesandbox repro?

so far we haven't faced this issue ourselves while using it in multiple scaffolded projects and consumer projects too and none of our consumers have also raised it.

@cseas
Copy link
Member Author

cseas commented Jan 17, 2023

Here's a reproduction:

Opening src/App.tsx in VSCode should show the warning.
Also npm run build will run fine regardless of what props you pass to Button.

Here's also a Codesandbox reproduction:
https://codesandbox.io/s/boring-aryabhata-vu8jc0?file=/src/App.tsx

I've tried installing other projects that ship with types and types for those get detected correctly. VSCode only shows this problem with Blade.

Here's a side-by-side comparison. TypeScript is able to find types for a library that ships with a "types" field and reports error when using a wrong prop. The same project setup can't detect types for Blade.

Screenshot 2023-01-17 at 8 31 48 PM

It looks like this bug occurs only when the project is using "module": "NodeNext" in tsconfig. That's probably why none of the Blade consumers have reported this yet. I'm guessing the new module resolution for ESM triggered by this config is looking for index.web.d.ts which it can't find. More on this topic here.

But as shown in the above screenshot, TypeScript can figure out types when defined explicitly so relying on automatic detection is not a reliable long-term solution.

Related: microsoft/TypeScript#50058 (comment)

@cseas cseas changed the title TypeScript cannot find declaration files TypeScript cannot find declaration files with module nodenext Jan 17, 2023
@kamleshchandnani
Copy link
Collaborator

can you check once and post what version of TS server is set on your vscode?

@cseas
Copy link
Member Author

cseas commented Jan 18, 2023

v4.7.3

Screenshot 2023-01-18 at 1 36 49 PM

Switching to the workspace version v4.9.4 doesn't fix the issue.

@kamleshchandnani
Copy link
Collaborator

can you switch to 4.9? because we had added module-specific resolution for .native and .web extensions as that feature was released in 4.8 so that could be one issue

@cseas
Copy link
Member Author

cseas commented Jan 18, 2023

I did try v4.9.4, not working 😅

Screenshot 2023-01-18 at 4 46 33 PM

Tried reloading VSCode after switching TypeScript version too, issue persists.

@anuraghazra
Copy link
Member

@cseas I noticed you didn't specify moduleResolution in tsconfig, when you don't specify it and set module=NodeNext it defaults to Classic.

If you set moduleResolution: 'Node' it should work as expected, the Classic option is set as default for compatibility reasons with older versions of TS.

@cseas
Copy link
Member Author

cseas commented Jan 20, 2023

moduleResolution in tsconfig, when you don't specify it and set module=NodeNext it defaults to Classic.

No, moduleResolution takes the same value as module when module is set to NodeNext. So moduleResolution in the linked repro is NodeNext which is intended.

Screenshot 2023-01-20 at 1 25 25 PM

Changing the module or moduleResolution to an option which transpiles to CommonJs would indeed resolve the issue. But that changes our bundle output to CommonJS instead of ESM, which is undesired.

More on this: How can I make my TypeScript project output ESM?

Also, as pointed out in this comment, TypeScript finds declaration files correctly for other UI libraries. So the problem isn't the setup/config here, the issue is that Blade is not supporting a setup that wants to bundle as ESM.

@anuraghazra
Copy link
Member

anuraghazra commented Jan 20, 2023

Okay I see, you want a pure ESM output, then in that case we can check if types field resolves the issue, but we need to see if it can work with moduleSuffixes since it needs to work seamlessly across RN and web.

@cseas
Copy link
Member Author

cseas commented Jan 20, 2023

Yes, the RN support is what I'm not sure about. The solution I suggested in the issue description will solve it for web projects but that could possibly break the types for RN.

As an alternate solution, maybe if we just rename index.d.ts to index.web.d.ts, that might fix it without breaking any RN setup. Here's the full error from VSCode:

Could not find a declaration file for module '@razorpay/blade/components'.
'/Users/abhijeet.singh/Documents/cseas/repro/node_modules/@razorpay/
blade/build/components/index.web.js' implicitly has an 'any' type.

Update: ^ Just tried this fix manually. It works for module: NodeNext but apparently breaks for moduleResolution: node. I figured out another alternate solution by modifying package exports. Sent a PR.

@kamleshchandnani
Copy link
Collaborator

@cseas from the codesandbox repo you sent above I see things are working fine 🤔
Played around with different values for module and it works in all cases even tried without changing anything on the link you have attached

Screenshot 2023-01-25 at 11 26 14 AM

@cseas
Copy link
Member Author

cseas commented Jan 25, 2023

My bad. I was playing around with the tsconfig values to debug another issue. Didn't realise Codesandbox saved it. Try now, the issue should come up.

Here's the tsconfig that reproduces the issue:

{
    "compilerOptions": {
        "jsx": "react-jsx",
        "module": "NodeNext"
    }
}

@kamleshchandnani
Copy link
Collaborator

Another question: to make this work as per your doc link type: module should be set in target of blade but we haven't set that so right now with your fix anyways things might break while bundling if the consumer project sets type: module in their package.json because now its misleading since our exports field is only pointing to .js version even though the output in the .js files is esm but its not following the file extension. so this might work for this particular use case but I'm not sure if this will work flawlessly for projects which set type: module in their package.json 🤔

@cseas
Copy link
Member Author

cseas commented Jan 25, 2023

That's correct, ESM version of libs should either ship with the .mjs extension or with type: module for node to recognise it as ESM. Not sure how it's working right now, need to investigate more on it.

My assumption is that there's no issues because the exports field in Blade's package.json directly points to the single flat file that contains everything a user can import from Blade. The module resolution of node will probably only start throwing errors on relative imports once Blade starts shipping the original tree instead of a single file (required to solve #959).

The GitHub repro I linked above has type: module. I manually added the changes from my linked PR to Blade under node_modules and tested. Just added type: module to the linked Codesandbox and that works too. So bundle looks fine. The only problem right now is TypeScript declarations not being found.

Related discussion: microsoft/TypeScript#18442 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants