Skip to content

Update some assets deps and react-router v6 -> v7 - #6820

Draft
apata wants to merge 1 commit into
masterfrom
update-assets-deps
Draft

apata wants to merge 1 commit into
masterfrom
update-assets-deps

Conversation

@apata

@apata apata commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Changes

Ran npm --prefix assets audit --fix, then updated react-router one major version, from 6 to 7. Latest major is 8, which requires react update as well, which we'll do separately.

Tests

  • This PR does not require tests

Changelog

  • This PR does not make a user-facing change

Documentation

  • This change does not need a documentation update

Dark mode

  • This PR does not change the UI

@apata apata added the preview label Oct 28, 2026
@github-actions

Copy link
Copy Markdown
Preview environment👷🏼‍♀️🏗️
PR-6820

Comment on lines +182 to +188
// react-router applies the URL change in a transition, so the draft must
// be cleared in one too, or the filter disappears between the two commits
startTransition(() => {
setEditing({
...current,
draft: null,
hasPushedHistoryEntry: true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This fix looks flimsier than I'd like, but manual testing confirms that it's working. We have previously talked about refactoring this module to do less, but haven't gotten to it, so maybe it's acceptable to have this hacky fix until we do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's another case right above (the setEditing call in the if (!hasValues(filter)) branch, line 167) that should be wrapped with startTransition:

  1. Add a "Source is Google" filter
  2. Open its value picker
  3. De-select Google
  4. Notice how a second pill flickers next to the one we're currently editing

I agree that it's acceptable as a patch, but would be nice to fix it properly by making sure the filters pills are rendered based on a single source of truth.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah damn, thanks for finding that! Let's do it right then.

@apata
apata requested a review from a team October 28, 2026 16:42
@apata
apata marked this pull request as draft October 29, 2026 12:21
@apata apata removed the preview label Oct 29, 2026

This branch has not been deployed

No deployments
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