Skip to content

fix(core): Adds express/fastify to peer dependency list and makes them optional#1008

Merged
kamilmysliwiec merged 2 commits into
nestjs:masterfrom
kamronbatman:kbatman/yarn_pnp_workspace
Feb 9, 2023
Merged

fix(core): Adds express/fastify to peer dependency list and makes them optional#1008
kamilmysliwiec merged 2 commits into
nestjs:masterfrom
kamronbatman:kbatman/yarn_pnp_workspace

Conversation

@kamronbatman

@kamronbatman kamronbatman commented Oct 7, 2022

Copy link
Copy Markdown
Contributor

Adds express/fastify to the peer dependency list to fix a problem with yarn pnp workspaces and resolving dependencies from @nestjs/common through @nestjs/serve-static.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

fix: Fixes a Nestjs runtime error saying it cannot find express when using yarn pnp workspaces.

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Receive the error: "The "express" package is missing. Please, make sure to install this library ($ npm install express) to take advantage of ServeStaticModule."

Screen Shot 2022-10-07 at 1 53 13 PM

Issue Number: N/A

What is the new behavior?

The ServeStaticModule loads properly and this error does not show up.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Adds express to peer dependency to fix a problem with yarn pnp workspaces and resolving dependencies from @nestjs/common through @nestjs/serve-static.
@kamronbatman

Copy link
Copy Markdown
Contributor Author

@kamilmysliwiec, since this is my first PR, I am not sure if I filled it out correctly. Let me know if I missed anything.

Comment thread package.json Outdated
"@nestjs/common": "^9.0.0",
"@nestjs/core": "^9.0.0"
"@nestjs/core": "^9.0.0",
"express": "^4.8.1"

@micalevisk micalevisk Oct 7, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

express is optional here because this package could be used with fastiy.

You could add fastify as well and mark both of them as optional with https://docs.npmjs.com/cli/v8/configuring-npm/package-json#peerdependenciesmeta

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea. I'll do that and test it out with yarn pnp workspaces.

@kamronbatman kamronbatman Oct 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@micalevisk I updated with the following:

  • I had the wrong express version, so fixed that.
  • Added fastify and fastify/static.
  • Added all 3 to peerDependenciesMeta so they are optional

As far as I can tell it is working well when I consume it using yarn v3.2.4 + pnp.

@kamronbatman kamronbatman changed the title fix(core): Adds express to peer dependency list fix(core): Adds express/fastify to peer dependency list and makes them optional Oct 10, 2022
@kamilmysliwiec kamilmysliwiec merged commit dc96f7d into nestjs:master Feb 9, 2023
@kamilmysliwiec

Copy link
Copy Markdown
Member

lgtm

@kamronbatman kamronbatman deleted the kbatman/yarn_pnp_workspace branch February 10, 2023 02:30
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