-
Notifications
You must be signed in to change notification settings - Fork 135
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 Automatic cdn invalidation #665
base: main
Are you sure you want to change the base?
Feat Automatic cdn invalidation #665
Conversation
commit: |
//TODO: test the constructed paths | ||
const constructedPaths = paths.flatMap(({ path, isAppRouter }) => | ||
isAppRouter | ||
? [`${path}`, `${path}?_rsc=*`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think only ${path}*
is enough here? I will do some testing tommorow btw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's on purpose ${path}*
would invalidate every route that start with path
. For example if you have a products
and a products/[slug]
route (using ${path}*
) it would be impossible to invalidate only products
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it also invalidates the products
if you use the ${path}*
one actually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not what i'm saying, it invalidates products
but also every single products/[slug]
route, which may not be what you want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but yeah im guessing we dont want that. its probably wise to only invalidate the path.
in my projects im just used to doing the star, cause normally you would want products
and products/[slug]
invalidated at the same time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The paths need to have a leading /
else an error will be thrown from cloudfront client: Failed to revalidate tag InvalidArgument: Your request contains one or more invalid invalidation paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sommeeeer It's only for revalidateTag
right ? You didn't test res.revalidate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only for
revalidateTag
right ? You didn't testres.revalidate
?
i tested revalidatePath('/products');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works:
const constructedPaths = paths.flatMap(({ path, isAppRouter }) =>
isAppRouter
? [`/${path}`, `/${path}?_rsc=*`]
: [
`/${path}`,
`/_next/data/${process.env.BUILD_ID}${path === "/" ? "/index" : path}.json*`,
],
);
|
Fix #658
Still a bit of work to do on this one, it will require a refactor of
InternalEvent
(orRoutingResult
) and the routingHandler so that we have access to the information we need (Or maybe we can use the async hook context)