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

Invalid type definitions - static import of ESM types from CJS types #447

Open
benmccann opened this issue Dec 1, 2023 · 5 comments
Open
Labels
type: bug code to address defects in shipped code

Comments

@benmccann
Copy link

benmccann commented Dec 1, 2023

Describe the bug

../../node_modules/.pnpm/@netlify+functions@2.4.0/node_modules/@netlify/functions/dist/function/v2.d.ts:1:30 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@netlify/serverless-functions-api")' call instead.

1 export type { Context } from '@netlify/serverless-functions-api';
                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error in ../../node_modules/.pnpm/@netlify+functions@2.4.0/node_modules/@netlify/functions/dist/function/v2.d.ts:1

Configuration

Import the library in a project which has the following in the tsconfig.json:

		"module": "node16",
		"moduleResolution": "node16",

This issue can be worked around with "skipLibCheck": true, so ensure that is not set when reproducing. It is not set by default.

@benmccann benmccann added the type: bug code to address defects in shipped code label Dec 1, 2023
@Skn0tt
Copy link
Contributor

Skn0tt commented Dec 5, 2023

Thanks for reporting this! It's good to know there's a workaround, i'll add it to our internal triage.

@Skn0tt
Copy link
Contributor

Skn0tt commented Dec 5, 2023

I'm a bit surprised about TypeScript emitting this error for a .d.ts file. That can't use require anyways, right?

@benmccann
Copy link
Author

They're not using require, but you can only import CJS from ESM if you use a dynamic import. Another possible solution besides updating the types would be to distribute this package as ESM.

@Skn0tt
Copy link
Contributor

Skn0tt commented Dec 5, 2023

They're not using require, but you can only import CJS from ESM if you use a dynamic import.

We're only importing @netlify/serverless-functions-api for types:

https://github.com/netlify/functions/blob/main/src/function/v2.ts#L1

So I don't think that an actual import line shows up in the generated JS file. This seems to be purely about the declaration file that TypeScript generates, and I would have expected TypeScript to generate it in a way that's compliant with CJS, because that's how it's configured:

"module": "commonjs" /* Specify what module code is generated. */,

Anyways, distributing this as ESM makes sense to me. @eduardoboucas do you see any reason why this might become a problem? Maybe consumption from within Next.js sites?

@ascorbic
Copy link
Contributor

I think this will be fixed by #473

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

No branches or pull requests

3 participants