Skip to content
This repository was archived by the owner on Apr 6, 2023. It is now read-only.
/frameworkPublic archive

docs(config): improve serverMiddleware jsdocs - #4594

Closed
danielroe wants to merge 2 commits into
mainfrom
docs/smw
Closed

docs(config): improve serverMiddleware jsdocs#4594
danielroe wants to merge 2 commits into
mainfrom
docs/smw

Conversation

@danielroe

Copy link
Copy Markdown
Member

🔗 Linked issue

resolvesnuxt/nuxt#13813

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

This removes some of the old data from serverMiddleware docs. I feel this could be considerably improved, maybe incorporating some of the nice new API docs... Or adding a notice suggesting using auto-detected ~/server/api etc...?

cc: @clemcode

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroedanielroe added the documentation Improvements or additions to documentation label Apr 25, 2022
@danielroe
danielroe requested a review from pi0April 25, 2022 13:39
@danielroedanielroe self-assigned this Apr 25, 2022
@netlify

netlifyBot commented Apr 25, 2022

Copy link
Copy Markdown

Deploy Preview for nuxt3-docs ready!

NameLink
🔨 Latest commit4aa6d3f
🔍 Latest deploy loghttps://app.netlify.com/sites/nuxt3-docs/deploys/6267c4edba5a8b0008452bb7
😎 Deploy Previewhttps://deploy-preview-4594--nuxt3-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

* { path: '/server-middleware/**', handler: '~/server-middleware/index.ts' },
* // We can create custom instances too, but only in development mode, they are ignored for the production bundle.
* { path: '/static2', handler: serveStatic(fileURLToPath(new URL('./static2', import.meta.url))) }
* { path: '/static2/**', handler: serveStatic(fileURLToPath(new URL('./static2', import.meta.url))) }

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.

I would rather not to advocate routing features alongside with legacy server middleware

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

then let's remove this entirely, as it won't work without the change.

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.

Okay right it seems a bug that we are broke with new handlers for legacy middleware.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Comment threadpackages/schema/src/config/_common.ts Outdated
* @example
* ```js
* export default function (req, res, next) {
* export default eventHandler(event => {

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.

server-middleware option is basically a legacy option I would mark as deprecated soon. We don't need to update it's example with something it is not mean to be. (BTW for consistency we use defineEventHandler across Nuxt docs)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Hm. I don't like leaving a legacy example. Shall we just remove and link to https://v3.nuxtjs.org/guide/features/server-routes#server-middleware instead?

@pi0pi0Apr 26, 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.

Me either but this IS a legacy option with expected (req, res) syntax :D Let's introduce new options and deprecate this.

@pi0pi0 changed the title docs: slightly improve smw infodocs(config): improve serverMiddleware jsdocsApr 25, 2022
Co-authored-by: pooya parsa <pyapar@gmail.com>
@pi0pi0 added the pending label Apr 26, 2022
@pi0

pi0 commented May 6, 2022

Copy link
Copy Markdown
Member

With #4850 this change is not relavant anymore as we register legacy serverMiddleware exactly as middleware not route handlers. Also deprecated by #4850 and removed from the docs.

@pi0pi0 closed this May 6, 2022
@danielroe
danielroe deleted the docs/smw branch May 6, 2022 15:00
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

3.xdocumentationImprovements or additions to documentationpending

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server middleware not working in 3.0.0-rc.1

2 participants

@danielroe@pi0