Resolve conflicts and api changes from react-router-6-13 branch #4548
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…using the package-lock.json from beta and re-running npm@8.5.5 install
If any of these functions are called while router is still null, that means our setTimeout(()=>…) dependency injection didn't work. Let's make that show up as an error in the browser console, rather than silently failing to navigate.
I'd like to make sure whenever we update this library, we do it as a focused and intentional endeavor, where someone gets to read the patch notes, and decide how much manual QA is needed. Patch notes for react-router: - https://github.com/remix-run/react-router/releases That might be true even if we weren't using `unstable_` API's. An alternative would be to use `~6.14.1` to stay on `6.14.x`, allowing some `npm` commands to sometimes update patch versions if they exist. --- Package-lock.json already specifies 6.14.1 prior to this commit, because it updated while I was resolving the merge conflict (6.14.1 qualifies as ^6.13.0).
Bump `pip-tools` version to 7.x.x
Language selector had an extra box outline introduced by #4207 and the click height was short. (UI fix not related to current PR, this came up while testing the form processing route.)
Superceded by /forms/:uid/settings/media
formatting-only commit
- Manually hoist variables to make scope clearer and use 'let' - Carefully replace 8 loose comparisons with strict ones (===, !==) - https://dorey.github.io/JavaScript-Equality-Table/ was useful here
It seems asset.content.translations can be undefined if you have a form with 0 questions, which is pretty uncommon but it leads to a full-page crash here. Fix the same crash when it appears in the data table settings menu, and on the data reports page.
…as it still has some impact on subroute behavior.
- jsapp/js/app.js - jsapp/js/components/drawer.es6 - jsapp/js/components/header/searchBoxStore.ts - jsapp/js/components/library/myLibraryStore.ts - jsapp/js/components/library/publicCollectionsStore.ts - jsapp/js/lists/forms.js - jsapp/js/mixins.tsx - jsapp/js/router/allRoutes.es6 - jsapp/js/components/map.es6 …in an attempt to reduce merge conflicts between diverging branches (beta after react router 6.14 upgrade, feature/my-projects)
jnm: > 0051_add_deployment_status_to_asset will have to be renamed to 0052… > and have its dependencies updated to the new 0051_set_free… that's > already in beta. > > Before doing this, people working on feature/my-projects should roll > their database back to 0050 with `./manage.py migrate kpi 0050`. After > the migration shuffle is done, then migrate (forward) normally: > ./manage.py migrate
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Integrate changes from the react-router 6.14 upgrade into the
feature/my-projectsbranch.Details
Since
feature/my-projectsis long-lived and contains its own modifications to routing, the merge with beta after #4505 has to be executed carefully.Notes for reviewer
I'd like @magicznyleszek to review this merge-conflict-resolution as the author of
feature/my-projects.The "Files changed" view will bury the most important manual merge decisions amongst large diffs that come directly from #4505. To try to create a better view, and highlight some of the important changes, I created TMP-naive-merge-for-comparison...merge-my-projects-react-router-6-14. (It's not perfect but it's better than the default view.)
I'm reasonably confident in the quality of this merge, but it's possible there's something subtle that's missed. Let's:
feature/my-projectslands inbeta.link to internal chat thread
Related issues
Caused by #4505 — Upgrade to React Router 6.14 and use upstream usePrompt
Needed for #4535 — Typescriptize MainHeader component