examples icon indicating copy to clipboard operation
examples copied to clipboard

Update middleware example

Open mattschwarz opened this issue 2 years ago • 8 comments

Description

Add common exclusions the URL rewrite to maintain page rendering capabilities and asset paths even in maintenance mode. Referencing this discussion: https://github.com/vercel/next.js/discussions/38615

Demo URL

Type of Change

  • [ ] New Example
  • [x] Example updates (Bug fixes, new features, etc.)
  • [ ] Other (changes to the codebase, but not to examples)

New Example Checklist

  • [ ] 🛫 npm run new-example was used to create the example
  • [x] 📚 The template wasn't used but I carefuly read the Adding a new example steps and implemented them in the example
  • [ ] 📱 Is it responsive? Are mobile and tablets considered?

mattschwarz avatar Feb 17 '23 20:02 mattschwarz

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
edge-api-routes-cache-control ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-api-routes-hello-world ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-api-routes-json-response ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-api-routes-query-parameters ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-functions-authed-proxy ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-functions-cookies ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-functions-news ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
edge-user-agent-based-rendering ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
example-reduce-image-bandwidth-usage ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
i18n ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-combining-data-fetching-strategies ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-image-fallback ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-image-offset ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-loading-web-fonts ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-microfrontends ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-microfrontends-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-pagination-with-ssg ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-reuse-responses ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-script-component-ad ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)
solutions-script-component-strategies ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 17, 2023 at 8:05PM (UTC)

vercel[bot] avatar Feb 17 '23 20:02 vercel[bot]

@mattschwarz is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] avatar Feb 17 '23 20:02 vercel[bot]

As far as I'm concerned, this change might be redundant since the middleware function is already limited to running only on specific paths by the config.matcher. Are you trying to prevent people from blindly copying code into their middleware that will run on all traffic?

dominiksipowicz avatar Feb 28 '23 17:02 dominiksipowicz

As far as I'm concerned, this change might be redundant since the middleware function is already limited to running only on specific paths by the config.matcher. Are you trying to prevent people from blindly copying code into their middleware that will run on all traffic?

Given the context of this specific example, and if you were to follow this example as-is, you'd most likely find that any maintenance page you create (assuming it's using any referenced dependencies like components, assets, etc) would be blocked resulting in a broken page...including things some basic things that I'd assume many may not consider, like your favicon. Maybe this could be a readme update to the example instead, but I think it's worth calling out that code like this will block all paths.

I do also think there's value in showcasing that there are certain paths you may want to leave open to traffic even when a site is in maintenance mode. For instance, paths your hosting on your app to assets referenced externally may not be links you want to break when taking a site down via something as quick/easy as an external config.

mattschwarz avatar Feb 28 '23 20:02 mattschwarz

I see. So it might be better to refactor this example to run on all paths and then exclude assets like you did.

WDYT @lfades @goncy ?

dominiksipowicz avatar Mar 01 '23 12:03 dominiksipowicz

In the current rewrite all static assets will also get rewritten to the assets of the maintenance page, which is intended behavior. It can't just be the browser URL and it won't include assets like those in the public folder unless this line explicitly includes them:

const isInMaintenanceMode = await get<boolean>('isInMaintenanceMode')

It currently does and we should change that, but once that's changed the behavior of also rewriting the static files should persist. @goncy can you check this one out as I might be missing something 🙏

lfades avatar Mar 01 '23 21:03 lfades

Hey @mattschwarz thanks for the PR! I get your point that sometimes when you go into maintenance mode, some "parts" of your app, like assets or static files can still be accessed. In this case we are showing how to get just one part of your app in maintenance mode, but I think we can add a section to the example saying that if you want to put your entire app in maintenance mode you should take care of excluding the assets and internal files from middleware. In that case we can leave a minimal example working but still covering your case, what do you think?

goncy avatar Mar 06 '23 16:03 goncy

Hey @mattschwarz thanks for the PR! I get your point that sometimes when you go into maintenance mode, some "parts" of your app, like assets or static files can still be accessed. In this case we are showing how to get just one part of your app in maintenance mode, but I think we can add a section to the example saying that if you want to put your entire app in maintenance mode you should take care of excluding the assets and internal files from middleware. In that case we can leave a minimal example working but still covering your case, what do you think?

Yeah I don't necessarily think we need to include my code change since the current example specifies a config matcher, but considering that this is the only example offered of implementing some kind of maintenance mode, I think it's a reasonable assumption that most would benefit from instruction on how to do that for an entire app. With that in mind, mentioning how and what paths to avoid accidentally targeting to keep critical links and functionality for the page itself working would be nice.

mattschwarz avatar Mar 07 '23 00:03 mattschwarz