Update to helmet v4

[mcollina]

Fixes fastify/help#225

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows Code of conduct

[EvanHahn]

If relevant, there are some other headers that are set by default. You can see that tested in Helmet's source code.

[mcollina]

Thanks!

[mcollina]

@EvanHahn Do you think you could export the HelmetOptions interface? So I can simplify https://github.com/fastify/fastify-helmet/blob/master/index.d.ts.

[EvanHahn]

Does the Parameters<typeof helmet> type work for you? If not, I can export it and publish a new release candidate.

[mcollina]

I don't think it would work. I'm getting Readonly<HelmetOptions> | undefined as the first parameter of helmet, however It does not satisfy the other constraints for FastifyPluginOptions.

   ✖  14:43  Type Readonly<HelmetOptions> | undefined does not satisfy the constraint FastifyPluginOptions.
  Type undefined is not assignable to type FastifyPluginOptions.

cc @Ethan-Arrowood

[mcollina]

Can you export that type as well?

[mcollina]

Overall, I have no idea on how to reuse the types from helmet, so any help on this front would be handy.

[Ethan-Arrowood]

Does HelmetOptions extend from FastifyPluginOptions? If not it won't be able to satisfy that constraint (unless you separately copy all the types in FastifyPluginOptions). I haven't looked past just this comment so there may be more details to the solution here.

[mcollina]

As I said, I have no clue about what I'm doing :). If you could push to this branch @Ethan-Arrowood that would be amazing. The types for the options are already fully typed in helmet, so we should be able to reuse that, shouldn't we?

[Ethan-Arrowood]

I'll take a stab now

[Ethan-Arrowood]

@EvanHahn yes please export the option interface!

[EvanHahn]

Will do! I'll plan to release a new release candidate later today.

[EvanHahn]

Just published [email protected], which you should be able to install with npm install helmet@next.

[Ethan-Arrowood]

Okay so I've done the given type change (which I believe is correct from looking at index.js). But as expected the test file is blowing up. Im assuming the api for helmet has changed in this major? Tomorrow I can look at fixing the test file but I don't totally see the purpose of all the tests if we are strictly dependent on helmet now. The tests would be more valuable on helmet side (might be a good cross project collaboration or good-first-issue on Evan's project)

[Ethan-Arrowood]

You can see the sort of change necessary in the diff from commit 60d9c3a

[EvanHahn]

I don't think I fully understand the problem, but Helmet 4 introduced some breaking changes which you can see in the changelog.

[Ethan-Arrowood]

Nothing to worry about Evan. The new helmet types are 💯

Either our previous types were a little inaccurate or some things have changed. I only know this because we have a file for testing our type definitions (powered by the tsd library). To complete this PR we need to update that test file so it correctly follows your types.

I'll be able to work on this more tomorrow

[delvedor]

LGTM