@redwoodjs/history #2212

Tobbe · 2021-04-04T14:02:35Z

The Problem

Some auth providers want to redirect the user when authorizing. They do this by a standard window.history.replaceState call (or global.history.replaceState). The address in the browser's address bar is updated, but the user isn't actually redirected.

The Cause

A standard RW app might look something like this

https://github.com/redwoodjs/redwood-tutorial/blob/3b59bc12ca211688839b395b8d1364a4c111db26/web/src/App.js#L15-L23

const App = () => (
  <FatalErrorBoundary page={FatalErrorPage}>
    <AuthProvider client={netlifyIdentity} type="netlify">
      <RedwoodApolloProvider>
        <Routes />
      </RedwoodApolloProvider>
    </AuthProvider>
  </FatalErrorBoundary>
)

Where <Routes> uses this code from the router

redwood/packages/router/src/router.tsx

Lines 213 to 225 in 4abbe45

    
           return ( 
        
             <RouterContextProvider 
        
               useAuth={useAuth} 
        
               paramTypes={paramTypes} 
        
               pageLoadingDelay={pageLoadingDelay} 
        
             > 
        
               <LocationProvider> 
        
                 <ParamsProvider> 
        
                   <RouteScanner>{children}</RouteScanner> 
        
                 </ParamsProvider> 
        
               </LocationProvider> 
        
             </RouterContextProvider> 
        
           )

A regular call to navigate looks like this

https://github.com/redwoodjs/redwood-tutorial/blob/2941c967adb6d7251ce3ae6e34edc3161e3fe89c/web/src/components/Post/Post.js#L36

navigate(routes.posts())

It will call this code where the address is updated (history.pushState) and all listeners are notified about the route change

redwood/packages/router/src/history.tsx

Lines 13 to 18 in 4abbe45

    
           navigate: (to: string) => { 
        
             global.history.pushState({}, '', to) 
        
             for (const listener of Object.values(listeners)) { 
        
               listener() 
        
             } 
        
           },

The listeners are located in <LocationProvider>. You can see that provider being used in the snippet from the router above. The provider's internal state is updated, and all its children are re-rendered. The router, being one of the children, sees the updated address and renders the new page ("posts" in this case).

This, below, is the problem

The direct call to just history.replaceState by the auth client bypasses the <LocationProvider> update, and so no re-rendering of the router happens.

The Solution

The solution might sound obvious. Just call the location listeners, like with a regular navigate() call.

We can't do this however, because the auth stuff is higher up in the hierarchy and so can't use the location context with its listeners.

My proposal

I propose that we introduce a new package, "history", that's responsible for keeping track of history, history changes and history change notification. This would be placed above auth in the hierarchy so auth can update the history (by calling history.replace), and the LocationProvider can keep listening to changes to history, and re-render its children when it changes.

Closes #1892

Adding some more context from #1892

This is what a typical App.js file looks like with auth0 added

const App = () => (
  <FatalErrorBoundary page={FatalErrorPage}>
    <AuthProvider client={auth0} type="auth0">
      <RedwoodApolloProvider>
        <Routes />
      </RedwoodApolloProvider>
    </AuthProvider>
  </FatalErrorBoundary>
)

When signing up you do something like this

<button
  onClick={() =>
    signUp({
      appState: { targetUrl: '/after-auth' },
    })
  }
>
  Sign In
</button>

When <AuthProvider> from the App snippet above is mounted it calls restoreAuthState, which in turn does this

restoreAuthState: async () => {
  if (
    global?.location?.search?.includes('code=') &&
    global?.location?.search?.includes('state=')
  ) {
    const { appState } = await client.handleRedirectCallback()
    history.replace(
      {},
      document.title,
      appState && appState.targetUrl
        ? appState.targetUrl
        : window.location.pathname
    )
  }
},

The problem is with the history.replace call. The router doesn't know about this (doesn't get any notifications). So the correct page isn't rendered. In fact, no rerender happens at all. So the user just stays on the current page. But the browser knows, so the URL is updated.

Here's a Github repo with a project that displays the issue https://github.com/tobbe/tobbe-redwood-history
Using the code in this PR the issue goes away. There's some info in the README in the repo on how to set it up.

github-actions · 2021-04-04T14:07:18Z

📦 PR Packages

Click to Show Package Download Links

Install this PR by running yarn rw upgrade --pr 2212:0.28.4-f207919

KrisCoulson · 2021-04-13T13:40:47Z

@Tobbe love this! I was thinking about picking up this smaller ticket but it might be worth lumping in with your new package. History back functionality: #1013

Tobbe · 2021-04-14T06:41:06Z

@KrisCoulson I'm not going to have time to look at this for a while. Do you want to pick it up instead?

sangheestyle · 2021-06-15T03:55:54Z

Hi @Tobbe ,

Can we make a separate PR for the jump first? I use global.history.replaceState() when I need it but it might be good if rw router have one for it. So the listeners can be used seamlessly.

redwood/packages/router/src/history.tsx

Line 28 in 2ced364

jump: (to: string, title: string = '') => {

Tobbe · 2021-06-15T09:05:07Z

Hi @Tobbe ,

Can we make a separate PR for the jump first? I use global.history.replaceState() when I need it but it might be good if rw router have one for it. So the listeners can be used seamlessly.

Hi @sangheestyle thanks for showing interest in this PR 🙂 I will spend some more time on this, and then discuss with the rest of the team about the best way forward.

sangheestyle · 2021-06-15T10:02:11Z

Hi @Tobbe ,
Can we make a separate PR for the jump first? I use global.history.replaceState() when I need it but it might be good if rw router have one for it. So the listeners can be used seamlessly.

Hi @sangheestyle thanks for showing interest in this PR 🙂 I will spend some more time on this, and then discuss with the rest of the team about the best way forward.

Awesome! Thanks for the reply!

Tobbe · 2021-06-15T21:31:03Z

@sangheestyle We decided to focus on getting this PR done as soon as possible instead of spending time on a separate PR for jump(). I don't want to make any promises, but we'll try to get this done this or next week.

thedavidprice · 2021-07-20T00:03:45Z

@jtoar confirming there are no breaking changes from this PR, correct? Also, is there anything we should do for documentation? Assuming "no".

jtoar · 2021-07-20T00:41:44Z

@thedavidprice no breaking changes. But ideally this PR will be superseded by #3055 before our next release (also no breaking changes).

We could add to the router documentation for the new replace option to navigate and the back function.

Tobbe mentioned this pull request Apr 4, 2021

Check for loading auth before rendering apollo provider #1945

Closed

thedavidprice marked this pull request as draft April 6, 2021 22:40

jtoar added topic/router-&-navigation hopper labels May 18, 2021

jtoar added this to hopper in Triage Jun 9, 2021

jtoar removed this from hopper in Triage Jun 10, 2021

jtoar added this to Backlog PRs in Current-Release-Sprint via automation Jun 10, 2021

jtoar added v1/priority and removed hopper labels Jun 10, 2021

jtoar moved this from Backlog PRs to On deck (future-release; help-wanted) in Current-Release-Sprint Jun 10, 2021

Tobbe force-pushed the tobbe-redwood-history branch 4 times, most recently from 0eba0b0 to 2ced364 Compare June 14, 2021 23:25

Tobbe force-pushed the tobbe-redwood-history branch 6 times, most recently from 93d3af0 to 0b6612d Compare June 15, 2021 09:01

Tobbe force-pushed the tobbe-redwood-history branch from 0b6612d to 4409be1 Compare June 15, 2021 09:05

Tobbe force-pushed the tobbe-redwood-history branch from ab153a0 to c1488a6 Compare June 15, 2021 21:21

thedavidprice moved this from On deck (help-wanted) to In progress (priority) in Current-Release-Sprint Jun 15, 2021

Tobbe added 13 commits July 15, 2021 10:21

Use gHistory in auth0

67d18a7

Same location check in jump as in navigate

2b49c6e

Include HistoryProvider in template

ea6d8d9

Bump history package version

ee4fb4e

Update test snapshots

353f632

Trim down HistoryProvider

2c5c053

eslint formatting

f4e23bf

Only restore auth once

94d8cb2

Include HistoryProvider in RedwoodProvider

92497bc

History: Remove HistoryProvider

fbf7c78

History: Add jump and back

9249e8a

navigation tests

59393df

history 35

d3114fb

Tobbe force-pushed the tobbe-redwood-history branch from ce9af26 to 98d0329 Compare July 15, 2021 09:18

Update router test

ee269c4

Tobbe force-pushed the tobbe-redwood-history branch from 98d0329 to ee269c4 Compare July 15, 2021 09:31

Tobbe and others added 7 commits July 15, 2021 11:42

Lint fixes

a8bbc94

rw template web: No need to explicitly depend on history

499f298

navigate options object

fe6d59a

navigate replace in auth0

28cd35a

auth0: move code around

3a20f49

Merge branch 'main' into tobbe-redwood-history

56faecf

Merge branch 'main' into tobbe-redwood-history

e320adf

jtoar merged commit 555b98c into redwoodjs:main Jul 15, 2021

Current-Release-Sprint automation moved this from Ready to merge to Done Jul 15, 2021

redwoodjs-bot bot modified the milestones: next-release-priority, next-release Jul 15, 2021

jtoar mentioned this pull request Jul 15, 2021

Reconsider how we restoreAuthState with auth0 #3048

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

@redwoodjs/history #2212

@redwoodjs/history #2212

Tobbe commented Apr 4, 2021 •

edited

github-actions bot commented Apr 4, 2021

KrisCoulson commented Apr 13, 2021

Tobbe commented Apr 14, 2021

sangheestyle commented Jun 15, 2021 •

edited

Tobbe commented Jun 15, 2021

sangheestyle commented Jun 15, 2021

Tobbe commented Jun 15, 2021

thedavidprice commented Jul 20, 2021

jtoar commented Jul 20, 2021 •

edited

	return (
	<RouterContextProvider
	useAuth={useAuth}
	paramTypes={paramTypes}
	pageLoadingDelay={pageLoadingDelay}
	>
	<LocationProvider>
	<ParamsProvider>
	<RouteScanner>{children}</RouteScanner>
	</ParamsProvider>
	</LocationProvider>
	</RouterContextProvider>
	)

	navigate: (to: string) => {
	global.history.pushState({}, '', to)
	for (const listener of Object.values(listeners)) {
	listener()
	}
	},

@redwoodjs/history #2212

@redwoodjs/history #2212

Conversation

Tobbe commented Apr 4, 2021 • edited

The Problem

The Cause

This, below, is the problem

The Solution

My proposal

github-actions bot commented Apr 4, 2021

📦 PR Packages

KrisCoulson commented Apr 13, 2021

Tobbe commented Apr 14, 2021

sangheestyle commented Jun 15, 2021 • edited

Tobbe commented Jun 15, 2021

sangheestyle commented Jun 15, 2021

Tobbe commented Jun 15, 2021

thedavidprice commented Jul 20, 2021

jtoar commented Jul 20, 2021 • edited

Tobbe commented Apr 4, 2021 •

edited

sangheestyle commented Jun 15, 2021 •

edited

jtoar commented Jul 20, 2021 •

edited