trezy / next-safe

https://trezy.gitbook.io/next-safe/
BSD 3-Clause "New" or "Revised" License
174 stars 20 forks source link

[BUG]: Types not correct #46

Open filipjakov opened 1 year ago

filipjakov commented 1 year ago

Describe the bug

In order to have typings from next.config, i use @ts-check at the top of the file.

I use next-safe in the following manner:

const nextSafe = require('next-safe');
const secureHeaders = nextSafe({
    isDev: process.env.NODE_ENV !== 'production',
    contentSecurityPolicy: false, // Will be dynamically set in the app
});

and get the following type error on the nextSafe call (but works in runtime since its the suggested way of usage):

(alias) module "next-safe"
import nextSafe
This expression is not callable.
  Type 'typeof import("next-safe")' has no call signatures.

The types suggest that this is possible:

const { nextSafe } = require('next-safe');

but this fails in runtime/build attempt.

Also, in order to have a dynamic CSP (depending on environment + current page), i use this package in _app.tsx and have the same problem.

Steps To Reproduce

  1. Create new Next.js app
  2. Add @ts-check in next.config.js or use csp in a .tsx component
  3. Setup next-safe in a recommended way
  4. Start app
  5. See error

Version

v3.x.x

Relevant log output

No response

Code of Conduct

sambauers commented 1 year ago

I've found that the addition of export = nextSafe to the generated types/types.d.ts module declaration fixes the initial issue:

declare module "next-safe" {
    function nextSafe(options: NextSafeConfig | undefined): Header[];
    export = nextSafe
}

I have tried to get that export line auto-generated through the JSDoc processor, but couldn't find a way.

But there is at least one further problem with the types once that's resolved… in lib/models/CSP.js, the CSPDirective definition needs to include false like so:

/**
 * A CSP Directive Poroperty
 * @typedef {(string|string[]|false)} CSPDirective
 */

I've started a conversion of this project to TypeScript and have found a couple of other types issues, so the above may not be everything that needs to be addressed.

sambauers commented 1 year ago

I've got a Typescript refactor sitting here now https://github.com/sambauers/next-safe/tree/sambauers/typescript - except for testing the release workflow (which I can't do) it's complete.

I'm not certain if I should create a PR for this though, what is the appetite for converting to Typescript @trezy?

trezy commented 1 year ago

@sambauers Feel free to make a PR... so long as you're game to come on as a core contributor and help maintain the types when I break them. 😝

sambauers commented 1 year ago

Pull request for Typescript refactor here #50

sambauers commented 1 year ago

@UncleClapton the initial beta release you created seems to have all the source code instead of just the compiled dist directory. I tried to configure semantic-release to only submit the dist directory by setting pkgRoot on the NPM plugin, but that doesn't seem to have done the job.

Ignore me. It did work.

sambauers commented 1 year ago

If anyone wants to test out the Typescript beta version…

UPDATED:

npm install next-safe@4.0.0-beta.2
sambauers commented 1 year ago

@UncleClapton there is a problem with the 4.0.0-beta.1 build where all the files have been placed in a src directory. I have a fix at #52 for you.

4.0.0-beta.1 is unusable, so you may want to remove that version from NPM.

sambauers commented 1 year ago

I've added beta to a live site here: https://rebeccabirch.au and output looks good.

The (partial) Next config is below (and @ts-check works now):

// @ts-check
/* eslint-disable @typescript-eslint/no-var-requires */

const { PHASE_DEVELOPMENT_SERVER } = require('next/constants')
const nextSafe = require('next-safe')

/**
 * @typedef NextConfig
 * @type {import('next').NextConfig}
 */

/**
 * @param {string} phase
 * @param {object} options
 * @param {NextConfig} options.defaultConfig
 * @returns {NextConfig}
 */
const nextConfig = (phase, { defaultConfig: _defaultConfig }) => {
  const isDevelopmentServer = phase === PHASE_DEVELOPMENT_SERVER

  return {
    i18n: {
      locales: ['en-AU'],
      defaultLocale: 'en-AU',
    },
    poweredByHeader: false,
    async headers() {
      return [
        {
          source: '/:path*',
          headers: [
            ...nextSafe({
              isDev: isDevelopmentServer,
              contentSecurityPolicy: {
                mergeDefaultDirectives: true,
                'child-src': ["'self'", 'data:', 'blob:'],
                'connect-src': ['ws:', 'wss:', 'https://vitals.vercel-insights.com'],
                'font-src': ['fonts.gstatic.com'],
                'img-src': ['data:'],
                'prefetch-src': false,
                'script-src': [
                  "'unsafe-eval'",
                  "'unsafe-inline'",
                  'https://www.googletagmanager.com/gtag/js',
                ],
                'style-src': ['fonts.googleapis.com', "'unsafe-inline'"],
                'worker-src': false,
              },
              referrerPolicy: 'strict-origin-when-cross-origin',
            }),
            {
              key: 'Strict-Transport-Security',
              value: 'max-age=63072000; includeSubDomains; preload',
            },
          ],
        },
      ]
    },
  }
}

module.exports = nextConfig