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

Add support for Drupal cache tags #797

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bojanbogdanovic
Copy link
Contributor

@bojanbogdanovic bojanbogdanovic commented Sep 7, 2024

This pull request is for: (mark with an "x")

  • examples/*
  • modules/next
  • packages/next-drupal
  • starters/basic-starter
  • starters/graphql-starter
  • starters/pages-starter
  • Other

GitHub Issue: #763

Copy link

vercel bot commented Sep 7, 2024

Someone is attempting to deploy a commit to the Chapter Three Team on Vercel.

A member of the Team first needs to authorize it.

Bojan Bogdanovic added 2 commits September 9, 2024 07:48
Removed spaces from next.services.yml
@minnur
Copy link

minnur commented Oct 1, 2024

Thanks for this PR @ bojanbogdanovic . Looks like the code is specific to nodes (nid). Can it be changed to be more generic ? Like instead of nid can we have something like entity_type_id entity_id so it works with any content type including custom?

@bojanbogdanovic
Copy link
Contributor Author

@minnur Yes this is possible, the only concern that I have is that the cache tags tables will grow, due the fact it will register more cache tags dependencies. The implementation is driven by JSON:API includes and focuses on actual pages to be revalidated on the frontend. I could introduce some kind property/flag on the NextEntity type to be able to specify which entity is used as a page. This way we can control the scope of the cache tags dependencies.

Copy link
Collaborator

@backlineint backlineint left a comment

Choose a reason for hiding this comment

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

Looking at this PR I've been confused as to why it is necessary to store a record of cache tags in the database at all.

Came to the conclusion today that the reason is that this is an implementation for the Pages router, while I've been thinking of it from the perspective of an implementation for the App router.

This might require some additional discussion / work outside of the context of this PR, so I added some further comments on the related issue: #763 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants