Skip to content

Conversation

cwangsmv
Copy link
Contributor

@cwangsmv cwangsmv commented Aug 25, 2025

Background

Current use-nunjucks hook which manages the Nunjuck rendering cache has the following issue:

  • Components are required to generate and provide their own unique IDs to store cache entries.
  • When new tags are created in the editor, the hook will create duplicate cache records instead of one.
  • Caches are cleared via setTimeout with a hard-coded interval.

Changes:

  • Components only need to pass the enableCache option to the hook in order to enable caching
  • Replace the global cache variable with a useRef-based cache, ensuring cache state is properly scoped to the hook instance and avoiding unique IDs.
  • Clear the render cache automatically whenever any environment (folder, collection, or global level) changes, preventing stale caches.
  • Add more meaningful inline comments to clarify the cache mechanism

Close INS-1248

@cwangsmv cwangsmv requested a review from jackkav August 25, 2025 09:08
@cwangsmv cwangsmv force-pushed the fix/simplify-nunjuck-caching branch from 1f80784 to 38f57e6 Compare August 25, 2025 09:40
*/
export const useNunjucks = (options?: UseNunjucksOptions) => {
// for all types of requests
const requestData = useRequestLoaderData();
// for request group (folder)
const { activeRequestGroup } = useRequestGroupLoaderData() || {};
const workspaceData = useWorkspaceLoaderData();
const { enableCache = false } = options || {};
const renderContextPromiseCache = useRef<Promise<BaseRenderContext>>();
Copy link
Contributor

Choose a reason for hiding this comment

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

There is still to much indirection here, Is it possible to just cache the renderContext rather than the promises, is seems like it would be easier to debug and understand. Can you document the effective lifespan of the cache, react ref lifespan is not clear.

@cwangsmv cwangsmv marked this pull request as draft September 3, 2025 02:43
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