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

chore: move @types/express to dependencies #714

Merged
merged 1 commit into from
Jun 17, 2020
Merged

chore: move @types/express to dependencies #714

merged 1 commit into from
Jun 17, 2020

Conversation

martinemmert
Copy link
Contributor

@martinemmert martinemmert commented May 13, 2020

We currently use this package within a package that is consumed by our applications through a shared runtime package (fig 1). Therefore our applications are not fond of express or next and don't include any types.

During the build process, Typescript will throw an error (fig 2) saying it can't find the types of express and advises us to install them.

// fig 1
// simplified dependency tree w/o node_modules/ directory
app-one
 └── next-runtime
  └── our-i18n-lib/
   └── next-i18next

app-two
 └── next-runtime
  └── our-i18n-lib/
   └── next-i18next
// fig 2
../../../node_modules/next-i18next/types.d.ts:11:25 - error TS7016: Could not find a declaration file for module 'express'. '/.../app-one/node_modules/express/index.js' implicitly has an 'any' type.
  Try `npm install @types/express` if it exists or add a new declaration (.d.ts) file containing `declare module 'express';`

11 import { Request } from 'express'
                           ~~~~~~~~~

After digging into this issue, we concluded that @types/express should be a regular dependency instead of a dev dependency.

The following two discussions do explain the reasons behind that:

I published a fork of this package with @types/express as regular dependencies for testing purposes here: https://www.npmjs.com/package/next-i18next_express-types

@martinemmert martinemmert changed the title deps: move @types/express to dependencies chore: move @types/express to dependencies May 13, 2020
@adrai
Copy link
Member

adrai commented Jun 13, 2020

is this still a problem with 5.0.0-beta.2 ? The express dependency should be gone with v5 🤷‍♂️

@isaachinman
Copy link
Contributor

@adrai Yep, that's correct. We don't have a clear picture of when v5 will land in prod, though.

@isaachinman isaachinman merged commit 41f93d6 into i18next:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants