-
Notifications
You must be signed in to change notification settings - Fork 368
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
Issues with TypeScript imports in 4.0.0 #235
Comments
What does your build setup look like? I didn't experience this problem when testing things, but I may have made a mistake. Also: are you using |
I'm using I believe this issue is universal. For example, create a import helmet from './index'
// import * as helmet from './index' confused TS and caused type error
console.log(helmet) It outputs |
A couple of questions to help me debug:
|
Seems like a TypeScript config issue. I just upgrades to helmet 4.0.0, the code is at https://github.com/Hongbo-Miao/hongbomiao.com/blob/master/server/src/security/middlewares/helmet.middleware.ts Hopefully it helps. |
I was able to resolve this by ignoring typescript errors before using import * as helmet from 'helmet'
...
// @ts-ignore
app.use(helmet(helmetConfig)) I believe this is because the default |
I'd like to fix the Helmet types. Helmet is a CommonJS module (in other words, it uses import helmet = require("helmet"); With import helmet from "helmet"; I'm not an expert at how TypeScript modules work, nor am I certain why some people are having problems and some are not. If anyone has suggestions for how to fix this, I'd appreciate it! |
To my understanding you would need to export helmet using This seems to be the correct way to handle CommonJS style exports in TypeScript: https://www.typescriptlang.org/docs/handbook/modules.html#export--and-import--require As far as I understand the concept, this should also still work correctly if importing projects use esModuleInterop=true. So hopefully not a breaking change. But that should be tested in some dummy project before publishing it to NPM as a minor/patch version. In the future it could be interesting to move away from exporting the function directly and use a named export instead (so As an alternative you could also look into creating a separate ESM build with a proper default export. But that would of course break existing Think for now sticking with pure CommonJS seems to be the best bet. |
+1 for named exports. They play nice with editor auto completion as well. |
Is there a way to do |
I got the same issue when I upgrade the helmet from 3.23.3 to 4.0 in my NestJS typescript project. any idea how to resolve this issue? |
re named exports - They're generally a good idea IMO. Since this is a major version, it's a good reason to justify the change to named exports? |
This should be fixed in the next version. Please try it out with |
import helmet = require('helmet');
// ...
app.use(helmet()); Tested with TypeScript
Looks good to me. Thanks! |
First thing is make it work, optimizing the type export is the next. :) |
@czzonet What do you mean? |
It is better if it can work like this? @EvanHahn import {helmet, HelmetOptions} from 'helmet' |
change the helmet version to this: |
@czzonet I don't think that's possible, unfortunately. It's arguably a breaking change, but it only affects TypeScript users where things weren't working well anyway. @pietrzakadrian What do you mean? Is there something wrong with Helmet, or are those instructions for other Helmet users? |
@EvanHahn After installing import helmet from 'helmet'
app.use(helmet(helmetConfig)) |
+1 for [email protected]. Typing issues are resolved. For anybody that was previously importing the import helmet = require('helmet');
const opts: Parameters<typeof helmet>[0] = {
// strongly typed helmet options go here...
};
helmet(opts); |
Planning to release |
This has been fixed in |
Works like a charm. For everyone running into issues, make sure to remove any earlier installations of |
You can also use import helmet from 'helmet';
app.use(helmet()); if you use |
In order to have the same behavior as before we had to import that interface like this: https://github.com/fastify/fastify-helmet/blob/6637767c0b3d088e0f8cf0b440112df4c48f9566/index.d.ts Said that I have to say that we had many problems with esModuleInterop in the fastify ecosystem, but we fixed all of them and now we support seamlessly TS, CJS and ESM, all with correct typings. Our solution was the "infamous triplet". You can read more about it here and here If you want to follow the same approach, I'll be happy to help you, if needed/wanted. |
Code like below overrides the exported object, which causes
default
being missing from every module.For example, inside
index.ts
, addconsole.log("wat", expectCt);
under the import, and we'll see it'sundefined
.The text was updated successfully, but these errors were encountered: