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

feat: HMR support for serverMiddleware #6881

Merged
merged 12 commits into from
Jan 19, 2020
Merged

feat: HMR support for serverMiddleware #6881

merged 12 commits into from
Jan 19, 2020

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Jan 16, 2020

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor

Description

Fixes #6877 #1509.

This PR brings huge DX improvements to Nuxt serverMiddleware by watching all serverMiddleware (and their require dependency tree). HMR supported for both path and handler.

Changes summary:

  • Server refactored to support in-place HMR of middleware stack and also adds additional meta (7505ac5)
    • New middleware syntax is more consistent with express (path/handler ~> route/handle - with backward compatibility)
  • Fixed an important bug in cjs helper that causes process fatal error when accessing a cache entry which had already errors (more DX coming in another PR for HMR of nuxt.config) (26581e6)
  • Replaced old full restart in Builder with new HMR in Builder (In rare cases if any error happens fullRestart fallback happens) (8a950ce)

Error handling: Normal runtime errors already being handled by error middleware. In case an error happens due to syntax/reference error, a placeholder will be automatically replaced. If path is defined inside same file, we try to preserve last state otherwise show error on all routes until problem is fixes.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

Copy link
Member

@atinux atinux left a comment

Choose a reason for hiding this comment

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

Small typo

packages/builder/src/builder.js Outdated Show resolved Hide resolved
packages/builder/src/builder.js Outdated Show resolved Hide resolved
@pi0 pi0 requested a review from atinux January 16, 2020 17:10
@atinux
Copy link
Member

atinux commented Jan 17, 2020

Can we add maybe a warn for path usage? Or update the docs for serverMiddleware?

@pi0
Copy link
Member Author

pi0 commented Jan 18, 2020

@atinux Internally we normalize path/handler without any warning by this PR to use connect/express convention. We can deprecate it for Nuxt3.

@GaborTorma
Copy link

GaborTorma commented Mar 21, 2020

Please update the docs with examples, because it not working for me.
Thanx!

@jgmullor
Copy link

I have the same issue as @GaborTorma, only the first change on the server middlewares is detected

@pi0
Copy link
Member Author

pi0 commented Mar 23, 2020

@GaborTorma @jgmullor

  • If you are using Windows, there is a known issue going to be fixed with next release.
  • This is a basic DX improvement for serverMiddleware. Nuxt functions coming soon ;)
  • Would you please create an issue with reproduction steps when it is not correctly working?

@jgmullor
Copy link

@pi0 in our case we're using OSX/Ubuntu, I will try to create an issue with a jsbin later today, thank you! =)

@HuddleHouse
Copy link

Does anybody have an example of this working?

I am getting this error:

this.nuxt.server.serverMiddlewarePaths is not a function

@pi0
Copy link
Member Author

pi0 commented Mar 30, 2020

@HuddleHouse Would you please ensure having 2.12+ version of @nuxt/server installed in node_modules? Are you using nuxt as middleware?

@HuddleHouse
Copy link

@pi0 Thanks for responding! That helped me figure out what was wrong.

It turns out 2.11+ versions of @nuxt/server & @nuxt/core were getting installed. I installed 2.12+ versions in dev dependencies and it worked!

@humanistical
Copy link

I'm getting the below error after this change, using an Express api backend exported as below:
~/api/index.js:
import express from 'express'
...
const app = express()
...
module.exports = {
route: '/api',
handler: app
}

nuxt.config.js:
serverMiddleware: [{ route: '/api', handler: '~/api/index.js' }]

Error:
FATAL Cannot create property '_middleware' on string '~/api/index.js'

at Server.resolveMiddleware (node_modules/@nuxt/server/dist/server.js:816:35)
at Server.useMiddleware (node_modules/@nuxt/server/dist/server.js:822:36)
at Server.setupMiddleware (node_modules/@nuxt/server/dist/server.js:680:12)
at async Server.ready (node_modules/@nuxt/server/dist/server.js:610:5)
at async Nuxt._init (node_modules/@nuxt/core/dist/core.js:689:7)

@mobihack
Copy link

@humanistical I am encountering the same problem. Any fix?

@TeleMediaCC
Copy link

I'm getting the below error after this change, using an Express api backend exported as below:
~/api/index.js:
import express from 'express'
...
const app = express()
...
module.exports = {
route: '/api',
handler: app
}

nuxt.config.js:
serverMiddleware: [{ route: '/api', handler: '~/api/index.js' }]

Error:
FATAL Cannot create property '_middleware' on string '~/api/index.js'

at Server.resolveMiddleware (node_modules/@nuxt/server/dist/server.js:816:35)
at Server.useMiddleware (node_modules/@nuxt/server/dist/server.js:822:36)
at Server.setupMiddleware (node_modules/@nuxt/server/dist/server.js:680:12)
at async Server.ready (node_modules/@nuxt/server/dist/server.js:610:5)
at async Nuxt._init (node_modules/@nuxt/core/dist/core.js:689:7)

@mobihack

You need to exports only the app in ~/api/index.js file:

module.exports = app

@mobihack
Copy link

mobihack commented Jul 18, 2020

@TeleMediaCC Thank you. This solved my problem.

For others who may encounter this problem, you don't need to sub route to '/api' inside your express setup. Just export without prepending api to your express routes.

Sample setup:

express /api/index.js

const express = require('express');
const app = express();

....

app.get('/auth', auth.getUser);
app.post('/auth', auth.checkUser);

....

module.exports = app;

In nuxt.config.js

module.exports = {
  serverMiddleware: [
    { route: '/api', handler: '~/api/index.js' }
  ]
};

This will generate automatically.

GET /api/auth
POST /api/auth

@danielroe danielroe added the 2.x label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't rebuild full Nuxt project on serverMiddleware changes
9 participants