Skip to content

fix: don't move files to the CDN if they match redirect/rewrite rules #832

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

Merged
merged 3 commits into from
Nov 24, 2021

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Nov 23, 2021

Summary

Ensures that if a file would match a redirect or beforeFiles rewrite, it isn;t served from the CDN as that would prevent the rule from being applied. We could potentially be smarter and check if we can represent the rule using Netlify redirects, but that would be quite complicated to achieve and wouldn't work for lots of scenarios

Test plan

  1. Visit these pages and ensure they redirect to the locale root: en fr
  2. Visit these pages and ensure that they show the image page, rather than an error message: en fr

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Fixes #819

image

@ascorbic ascorbic self-assigned this Nov 23, 2021
@github-actions github-actions bot added the type: bug code to address defects in shipped code label Nov 23, 2021
@ascorbic ascorbic changed the title fix: don't move files to the CDN if they match redirects or beforeFiles rewrites fix: don't move files to the CDN if they match redirect/rewrite rules Nov 23, 2021
@ascorbic ascorbic requested a review from a team November 23, 2021 13:38
@ascorbic ascorbic marked this pull request as ready for review November 23, 2021 13:39
Release-As: 4.0.0-beta.10
@@ -1,4 +1,4 @@
// @ts-check
/* eslint-disable max-lines */

Choose a reason for hiding this comment

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

[sand] We should probably avoid disabling eslint and separate this file into smaller ones moving forward

exports.matchMiddleware = matchMiddleware
exports.stripLocale = stripLocale
exports.isDynamicRoute = isDynamicRoute

// eslint-disable-next-line max-lines-per-function
Copy link

@tiffafoo tiffafoo Nov 24, 2021

Choose a reason for hiding this comment

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

[sand] Same for eslint

Copy link

@tiffafoo tiffafoo left a comment

Choose a reason for hiding this comment

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

LGTM!

@kodiakhq kodiakhq bot merged commit 9e3dd0e into main Nov 24, 2021
@kodiakhq kodiakhq bot deleted the mk/rewrite-static branch November 24, 2021 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid moving static pages that are the source of redirects
2 participants