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

Losing types when importing cjs module using default export with nodenext modules #48845

Closed
wight554 opened this issue Apr 25, 2022 · 7 comments
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@wight554
Copy link

Bug Report

🔎 Search Terms

cjs typescript nodenext default export

🕗 Version & Regression Information

4.7.0-dev

While using nodenext with pure cjs modules, such as https://github.com/fastify/fastify-cookie or https://github.com/fastify/fastify-static , types are lost when using default export and cjs named export is prohibited

⏯ Playground Link

Playground link with relevant code

💻 Code

import fastifyCookie from 'fastify-cookie';
import fastifyStatic from 'fastify-static';

app.register(fastifyCookie);
app.register(fastifyStatic);

🙁 Actual behavior

import fastifyCookie
Argument of type 'typeof import("/.../node_modules/fastify-cookie/plugin")' is not assignable to parameter of type 'FastifyPluginCallback<any, Server> | FastifyPluginAsync<any, Server> | Promise<{ default: FastifyPluginCallback<any, Server>; }> | Promise<...>'.ts(2345)

🙂 Expected behavior

Type should not be lost while import
Can be workaround by

import { default as fastifyCookie } from 'fastify-cookie';
import { default as fastifyStatic } from 'fastify-static';

But i'm not sure if that's expected or not

@andrewbranch
Copy link
Member

Is your file a CJS or ES module? (If you’re not 105% sure: what is its file extension and what is its nearest package.json’s "type" key?)

@wight554
Copy link
Author

Fastify plugins are definitely CJS, the project I'm importing those to is ESM (type: "module", module: "nodenext", moduleResolution: "nodenext")
You can see in this PR: wight554/blog-template#50

@wight554
Copy link
Author

wight554 commented Apr 26, 2022

When using named import, TS successfully compiles, but node throws error:

file:///.../blog-template/dist/server/server.js:9
import { fastifyStatic } from 'fastify-static';
         ^^^^^^^^^^^^^
SyntaxError: Named export 'fastifyStatic' not found. The requested module 'fastify-static' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'fastify-cookie';
const {fastifyCookie} = pkg;

It actually works when following example from error, still wonder if it's a typescript bug or not
At least additionally to issue with incorrectly typed default export, typescript doesn't prevent cjs named import from happening and allows to break runtime.
Module export looks like this:

// plugin.js

/**
 * These export configurations enable JS and TS developers
 * to consume fastify-cookie in whatever way best suits their needs.
 * Some examples of supported import syntax includes:
 * - `const fastifyCookie = require('fastify-cookie')`
 * - `const { fastifyCookie } = require('fastify-cookie')`
 * - `import * as fastifyCookie from 'fastify-cookie'`
 * - `import { fastifyCookie } from 'fastify-cookie'`
 * - `import fastifyCookie from 'fastify-cookie'`
 */
fastifyCookie.fastifyCookie = fastifyCookie
fastifyCookie.default = fastifyCookie
module.exports = fastifyCookie
// plugin.d.ts

declare const fastifyCookie: FastifyPluginCallback<NonNullable<FastifyCookieOptions>>;

export default fastifyCookie;
export { fastifyCookie };

@andrewbranch
Copy link
Member

Fastify plugins are definitely CJS

Yes they are, but their own types incorrectly declare a default export:

image

@andrewbranch andrewbranch added the External Relates to another program, environment, or user action which we cannot control. label Apr 26, 2022
@andrewbranch andrewbranch closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2022
@wight554
Copy link
Author

Fastify plugins are definitely CJS

Yes they are, but their own types incorrectly declare a default export:

image

I see #13626 is still open, so there's no way to specify type of default export
What's recommended way to do this for cjs modules?

@andrewbranch
Copy link
Member

#13626 is not related to this. Fastify just wrote wrong types. Unfortunately the only thing to do is open an issue, fork and fix, or @ts-ignore.

@wight554
Copy link
Author

#13626 is not related to this. Fastify just wrote wrong types. Unfortunately the only thing to do is open an issue, fork and fix, or @ts-ignore.

Uh, I see that in this case, the only "easy" fix is to support named import for fastify-static and follow nodejs error recommendation
Like this:

import cookiePlugin from 'fastify-cookie';
const { fastifyCookie } = cookiePlugin;

BTW, shouldn't TS compiler throw error when trying to make named import of CJS module like node does in runtime?

wight554 added a commit to wight554/vite-plugin-checker that referenced this issue May 25, 2022
* currently it shows error:
This expression is not callable.
  Type 'typeof import("node_modules/vite-plugin-checker/lib/main")' has no call signatures.ts(2349)
* see microsoft/TypeScript#48845
wight554 added a commit to wight554/fastify-cookie that referenced this issue May 25, 2022
wight554 added a commit to wight554/fastify-static that referenced this issue May 25, 2022
mcollina pushed a commit to fastify/fastify-cookie that referenced this issue Jul 22, 2022
* make type definitons `"module": "nodenext"` compatible

* see microsoft/TypeScript#48845

* fix test

* fix default export type

* make typing extra safe and small refactoring

* restore brackets

* add additional properties to named export and fix test suite

* remove formatting

* revert formatting

* reuse type
fi3ework pushed a commit to fi3ework/vite-plugin-checker that referenced this issue Jul 23, 2022
…#140)

* add named export to improve `"module": "nodenext"` compatibility

* currently it shows error:
This expression is not callable.
  Type 'typeof import("node_modules/vite-plugin-checker/lib/main")' has no call signatures.ts(2349)
* see microsoft/TypeScript#48845

* switch to cjs module style

* Revert "switch to cjs module style"

This reverts commit ef7a774.

* remove extra semicolon

* fix patch
Uzlopak pushed a commit to Uzlopak/fastify-cookie that referenced this issue Jul 25, 2022
* make type definitons `"module": "nodenext"` compatible

* see microsoft/TypeScript#48845

* fix test

* fix default export type

* make typing extra safe and small refactoring

* restore brackets

* add additional properties to named export and fix test suite

* remove formatting

* revert formatting

* reuse type
Uzlopak added a commit to fastify/fastify-cookie that referenced this issue Aug 11, 2022
* integrate cookie signer

* add algorithm as plugin-option

* check for matching error message

* improve tests

* remove unnecessary array conversion

* simplify

* fix comment

* restructure SignerFactory

* update typings

* fix exports

* add benchmarks

* improve perf for multi secrets

* rename SignerFactory to Signer

* Merge branch 'master' into integrate-cookie-signer

* export signerFactory

* Update signer.js

* add secure: 'auto' Option (#199)

* add secure: auto Option

* document deviation from cookie serialize

* describe better

* lowercase Lax

* Bumped v7.3.0

Signed-off-by: Matteo Collina <hello@matteocollina.com>

* make type definitons `"module": "nodenext"` compatible (#184)

* make type definitons `"module": "nodenext"` compatible

* see microsoft/TypeScript#48845

* fix test

* fix default export type

* make typing extra safe and small refactoring

* restore brackets

* add additional properties to named export and fix test suite

* remove formatting

* revert formatting

* reuse type

* Bumped v7.3.1

Signed-off-by: Matteo Collina <hello@matteocollina.com>

* modify types

* remove dummy

* fix regex bottleneck found by sonarqube

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Co-authored-by: Matteo Collina <hello@matteocollina.com>
Co-authored-by: Volodymyr Zhdanov <wight554@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

2 participants