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

TS1479, helmet typings issue #210

Closed
2 tasks done
Uzlopak opened this issue Nov 25, 2022 · 9 comments
Closed
2 tasks done

TS1479, helmet typings issue #210

Uzlopak opened this issue Nov 25, 2022 · 9 comments
Assignees

Comments

@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 25, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.10.2

Plugin version

10.1.0

Node.js version

16.17.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

22.04

Description

It seems that we get typescript errors, because the typings from helmet are ecmascript and not commonjs

uzlopak@Battlestation:~/workspace/nodenext-mvce$ npm run build

> nodenext-test@1.0.0 build
> tsc -p .

node_modules/@fastify/helmet/types/index.d.ts:2:62 - 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("helmet")' call instead.

2 import helmet, { contentSecurityPolicy, HelmetOptions } from 'helmet';
                                                               ~~~~~~~~


Found 1 error in node_modules/@fastify/helmet/types/index.d.ts:2

Steps to Reproduce

clone https://github.com/Uzlopak/nodenext-mvce/tree/helmet-issue
npm run build

Expected Behavior

Should not throw

@kibertoad
Copy link
Member

Would our fancy nodenext triplet help?

@kibertoad
Copy link
Member

OK, I see https://github.com/fastify/fastify-helmet/pull/207/files
is it actually the cause for the problem?
@climba03003 thoughts?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Nov 25, 2022

@kibertoad

with v10.0.2 and even earlier versions we get that error. Nothing todo with our nodenext changes.

@climba03003
Copy link
Member

climba03003 commented Nov 25, 2022

@kibertoad helmetjs/helmet#389 (comment)

TypeScript is blaming the mis-match of types harder and harder. It is clearly an upstream issue.

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Dec 1, 2022

Opened a PR to fix this issue upstream helmetjs/helmet#391

@ri5t3ai
Copy link

ri5t3ai commented Mar 18, 2023

confirming it still persists

node_modules/@fastify/helmet/types/index.d.ts:2:62 - 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("helmet")' call instead.

2 import helmet, { contentSecurityPolicy, HelmetOptions } from 'helmet';

@climba03003
Copy link
Member

climba03003 commented Mar 18, 2023

I am going to lock this issue.
We know the issue is on going but it cannot be fixed until helmet itself does the changes.

Please do not open a new issue for the same problem.

@fastify fastify locked as too heated and limited conversation to collaborators Mar 18, 2023
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Apr 8, 2023

@EvanHahn fixed this issue in v6.1.0 of helmet.

@Uzlopak Uzlopak closed this as completed Apr 8, 2023
@kibertoad
Copy link
Member

@Uzlopak should we bump minimum version?

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

No branches or pull requests

5 participants