Skip to content

Conversation

@Mbeaulne
Copy link
Collaborator

@Mbeaulne Mbeaulne commented Jan 27, 2026

Description

Resolves: https://github.com/Shopify/oasis-frontend/issues/456

Added a new useRunSearchParams hook to manage pipeline run search/filter state with URL synchronization. This hook provides functionality to parse, update, and serialize filter parameters in the URL, making it easier to maintain filter state across page refreshes and navigation.

Type of Change

  • New feature
  • Improvement

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Test Instructions

  1. Run the test suite to verify the hook's functionality: npm test src/hooks/useRunSearchParams.test.ts
  2. The hook will be used in upcoming pipeline run filtering components

Additional Comments

This PR also:

  • Adds a new .local/ directory to .gitignore for local notes and scratch files
  • Adds src/types/pipelineRunFilters.ts to knip.json ignored files
  • Enables React Compiler for the new hook

Copy link
Collaborator Author

Mbeaulne commented Jan 27, 2026

@Mbeaulne Mbeaulne changed the title Pipeline run filters - basic Add useRunSearchParams hook for pipeline run filtering Jan 27, 2026
@Mbeaulne Mbeaulne force-pushed the 01-27-pipeline_run_filters_-_basic branch from 1f60f2c to 57a994f Compare January 27, 2026 15:32
Comment on lines +139 to +151
const setFilterDebounced = <K extends keyof PipelineRunFilters>(
key: K,
value: PipelineRunFilters[K],
) => {
if (debounceTimeoutRef.current) {
clearTimeout(debounceTimeoutRef.current);
}

debounceTimeoutRef.current = setTimeout(() => {
setFilter(key, value);
debounceTimeoutRef.current = null;
}, DEBOUNCE_MS);
};
Copy link

Choose a reason for hiding this comment

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

Critical: Stale closure bug causes filter loss

setFilterDebounced captures the current setFilter function which has filters in its closure. If the URL changes between calling setFilterDebounced and the timeout firing (e.g., from another filter update), the stale filters will be used, causing data loss.

Scenario: User has status=RUNNING, calls setFilterDebounced('pipeline_name', 'test'), then immediately sets created_by=user1. When the debounced timeout fires, it uses the old filters without created_by, losing that filter.

Fix by reading fresh filters in the timeout callback:

const setFilterDebounced = <K extends keyof PipelineRunFilters>(
  key: K,
  value: PipelineRunFilters[K],
) => {
  if (debounceTimeoutRef.current) {
    clearTimeout(debounceTimeoutRef.current);
  }

  debounceTimeoutRef.current = setTimeout(() => {
    // Re-parse filters from current URL to avoid stale closure
    const currentFilterParam = isRecord(search)
      ? getStringOrUndefined(search.filter)
      : undefined;
    const currentFilters = parseFiltersFromUrl(currentFilterParam);
    
    const newFilters = { ...currentFilters };
    if (value === undefined || value === null || value === "") {
      delete newFilters[key];
    } else {
      newFilters[key] = value;
    }
    updateUrl(newFilters);
    debounceTimeoutRef.current = null;
  }, DEBOUNCE_MS);
};
Suggested change
const setFilterDebounced = <K extends keyof PipelineRunFilters>(
key: K,
value: PipelineRunFilters[K],
) => {
if (debounceTimeoutRef.current) {
clearTimeout(debounceTimeoutRef.current);
}
debounceTimeoutRef.current = setTimeout(() => {
setFilter(key, value);
debounceTimeoutRef.current = null;
}, DEBOUNCE_MS);
};
const setFilterDebounced = <K extends keyof PipelineRunFilters>(
key: K,
value: PipelineRunFilters[K],
) => {
if (debounceTimeoutRef.current) {
clearTimeout(debounceTimeoutRef.current);
}
debounceTimeoutRef.current = setTimeout(() => {
// Re-parse filters from current URL to avoid stale closure
const currentFilterParam = isRecord(search)
? getStringOrUndefined(search.filter)
: undefined;
const currentFilters = parseFiltersFromUrl(currentFilterParam);
const newFilters = { ...currentFilters };
if (value === undefined || value === null || value === "") {
delete newFilters[key];
} else {
newFilters[key] = value;
}
updateUrl(newFilters);
debounceTimeoutRef.current = null;
}, DEBOUNCE_MS);
};

Spotted by Graphite Agent

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@Mbeaulne Mbeaulne force-pushed the 01-27-pipeline_run_filters_-_basic branch from 8d921a9 to 5de01ff Compare January 28, 2026 20:46
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.

2 participants