search

expressjs / express

Fast, unopinionated, minimalist web framework for node.
https://expressjs.com
MIT License
65.69k stars 16.29k forks source link

Rendering with view engine fails with Yarn v2 workspaces #4481

Closed brandon-leapyear closed 3 years ago

brandon-leapyear commented 3 years ago

We're trying to upgrade to Yarn v2 in our project, which uses NestJS in a Yarn workspace. Yarn v2 adds Plug-n-Play which enforces dependencies explicitly list their dependencies. When running our server and calling an endpoint that renders a pug template, we get:

express tried to access pug, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.

Required package: pug (via "pug")
Required by: express@npm:4.17.1 (via /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/)

Require stack:
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/view.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/application.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/express.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/index.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/$$virtual/@nestjs-platform-express-virtual-71cd35a84f/0/cache/@nestjs-platform-express-npm-7.5.5-1f245a935a-8af8bba8dc.zip/node_modules/@nestjs/platform-express/adapters/express-adapter.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/$$virtual/@nestjs-platform-express-virtual-71cd35a84f/0/cache/@nestjs-platform-express-npm-7.5.5-1f245a935a-8af8bba8dc.zip/node_modules/@nestjs/platform-express/adapters/index.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/$$virtual/@nestjs-platform-express-virtual-71cd35a84f/0/cache/@nestjs-platform-express-npm-7.5.5-1f245a935a-8af8bba8dc.zip/node_modules/@nestjs/platform-express/index.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/unplugged/@nestjs-core-virtual-74f5478e67/node_modules/@nestjs/core/nest-factory.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/.yarn/unplugged/@nestjs-core-virtual-74f5478e67/node_modules/@nestjs/core/index.js
- /Users/bchinn/Desktop/yarn2/express-pug-error/foo/dist/main.js +9467ms

I was able to repro it below using just express, but it seems like this only happens in a workspace. If you run the yarn commands below in the foo directory (and just yarn node main.js) it works.

I am a little confused why it works without workspaces, but that's a separate question for Yarn (@arcanis if you want to step into this). It seems like the best solution is to have the user resolve the path to the view engine themselves; e.g. instead of

// 'pug' determines both default extension and default module to require
app.set('view engine', 'pug')

// renders index.pug, requiring 'pug'
res.render('index')

// renders index.foo, requiring 'foo'
res.render('index.foo')

do

// any engine the user wants to use to render needs to
// be defined here
// maps extension => module
app.set('view engines', {
  pug: require.resolve('pug'),
  foo: require.resolve('foo'),
})
// specifies the default extension
app.set('default view engine', 'pug')

// shortcut for
//   app.set('view engines', { pug: require.resolve('pug') })
//   app.set('default view engine', 'pug')
app.set('view engine', 'pug', require.resolve('pug'))

// renders index.pug, requiring 'require.resolve('pug')'
res.render('index')

// renders index.pug, requiring 'require.resolve('pug')'
res.render('index.pug')

// renders index.foo, requiring 'require.resolve('foo')'
res.render('index.foo')

Minimal Repro

  1. Download repro.tar.gz
    1. Contains a top-level package.json file that lists foo/ as a workspace
    2. Contains foo/ with main.js and package.json
  2. Extract in a new directory
  3. yarn set version berry (skip this step to use yarn v1, which works)
  4. yarn
  5. yarn workspace foo node main.js
  6. curl localhost:3000

Yarn v2 gets me

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>Error: express tried to access pug, but it isn&#39;t declared in its dependencies; this makes the require call ambiguous and unsound.<br><br>Required package: pug (via &quot;pug&quot;)<br>Required by: express@npm:4.17.1 (via /Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/)<br><br>Require stack:<br>- /Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/view.js<br>- /Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/application.js<br>- /Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/express.js<br>- /Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/index.js<br>- /Users/bchinn/Desktop/yarn2/foo/foo/main.js<br> &nbsp; &nbsp;at internalTools_makeError (/Users/bchinn/Desktop/yarn2/foo/.pnp.js:5784:34)<br> &nbsp; &nbsp;at resolveToUnqualified (/Users/bchinn/Desktop/yarn2/foo/.pnp.js:6749:23)<br> &nbsp; &nbsp;at resolveRequest (/Users/bchinn/Desktop/yarn2/foo/.pnp.js:6841:29)<br> &nbsp; &nbsp;at Object.resolveRequest (/Users/bchinn/Desktop/yarn2/foo/.pnp.js:6919:26)<br> &nbsp; &nbsp;at Function.external_module_.Module._resolveFilename (/Users/bchinn/Desktop/yarn2/foo/.pnp.js:6017:34)<br> &nbsp; &nbsp;at Function.external_module_.Module._load (/Users/bchinn/Desktop/yarn2/foo/.pnp.js:5882:48)<br> &nbsp; &nbsp;at Module.require (internal/modules/cjs/loader.js:965:19)<br> &nbsp; &nbsp;at require (internal/modules/cjs/helpers.js:88:18)<br> &nbsp; &nbsp;at new View (/Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/view.js:81:14)<br> &nbsp; &nbsp;at Function.render (/Users/bchinn/Desktop/yarn2/foo/.yarn/cache/express-npm-4.17.1-6815ee6bf9-c4b470d623.zip/node_modules/express/lib/application.js:570:12)</pre>
</body>
</html>

while Yarn v1 gets me

<p>hello world</p>
brandon-leapyear commented 3 years ago

:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:

Never mind, got it working with

app.engine('pug', require('pug').__express)

But especially as PnP gets more widespread, it seems like the current workflow of

app.engine('pug', require('pug').__express) // sets the engine for pug
app.engine('foo', require('foo').__express) // sets the engine for foo
app.set('view engine', 'pug') // sets the default

is a bit verbose and confusing (why does view engine set the default?). I'll close this for now, but I think this configuration workflow could be improved

arcanis commented 3 years ago

I am a little confused why it works without workspaces, but that's a separate question for Yarn (@arcanis if you want to step into this).

Cf this doc entry: https://yarnpkg.com/features/pnp#different-behaviours-based-on-workspace--not-a-workspace

It seems like the best solution is to have the user resolve the path to the view engine themselves

Indeed. To unblock you in the meantime, you can use packageExtensions to declare pug as a peer dependency of express. That will allow Express to require it, provided the parent package properly lists it.