Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hook to get the server context #737

Merged
merged 12 commits into from
Sep 30, 2022
Merged

Hook to get the server context #737

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eae5d77
1st attempt at porting PR #713
vmarta Sep 23, 2022
2e37984
Clarify React component vs element
vmarta Sep 26, 2022
980a147
Simplify the hook interface
vmarta Sep 26, 2022
b6c8d53
Some clean-up
vmarta Sep 26, 2022
d9aa427
Isolate mention of prepass to within `with-react-query`
vmarta Sep 29, 2022
4c33040
Not touching `res.locals`
vmarta Sep 29, 2022
9014a0e
Merge branch 'develop' into request-response-hook
vmarta Sep 29, 2022
4ccae9c
Merge some files together
vmarta Sep 29, 2022
3944f41
Revert changes to device-context.js
vmarta Sep 29, 2022
f258e9f
Add test
vmarta Sep 30, 2022
6e2e7bb
Remove the example code
vmarta Sep 30, 2022
94fbc55
Merge branch 'develop' into request-response-hook
vmarta Sep 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions packages/pwa-kit-react-sdk/src/ssr/browser/main.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React from 'react'
import ReactDOM from 'react-dom'
import {BrowserRouter as Router} from 'react-router-dom'
import DeviceContext from '../universal/device-context'
import {DeviceContext, ServerContext} from '../universal/contexts'
import App from '../universal/components/_app'
import {getAppConfig} from '../universal/compatibility'
import Switch from '../universal/components/switch'
Expand Down Expand Up @@ -74,18 +74,20 @@ export const start = () => {
.then(() => new Promise((resolve) => loadableReady(resolve)))
.then(() => {
ReactDOM.hydrate(
<Router>
<DeviceContext.Provider value={{type: window.__DEVICE_TYPE__}}>
<AppConfig locals={locals}>
<Switch
error={error}
appState={window.__PRELOADED_STATE__}
routes={routes}
App={WrappedApp}
/>
</AppConfig>
</DeviceContext.Provider>
</Router>,
<ServerContext.Provider value={{}}>
<Router>
<DeviceContext.Provider value={{type: window.__DEVICE_TYPE__}}>
<AppConfig locals={locals}>
<Switch
error={error}
appState={window.__PRELOADED_STATE__}
routes={routes}
App={WrappedApp}
/>
</AppConfig>
</DeviceContext.Provider>
</Router>
</ServerContext.Provider>,
rootEl,
() => {
window.__HYDRATING__ = false
Expand Down
31 changes: 22 additions & 9 deletions packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {StaticRouter as Router, matchPath} from 'react-router-dom'
import serialize from 'serialize-javascript'

import {getAssetUrl} from '../universal/utils'
import DeviceContext from '../universal/device-context'
import {DeviceContext, ServerContext} from '../universal/contexts'

import Document from '../universal/components/_document'
import App from '../universal/components/_app'
Expand Down Expand Up @@ -191,20 +191,33 @@ export const render = async (req, res, next) => {
}
}

const OuterApp = ({res, error, App, appState, routes, routerContext, location, deviceType}) => {
const OuterApp = ({
req,
res,
error,
App,
appState,
routes,
routerContext,
location,
deviceType
}) => {
const AppConfig = getAppConfig()
return (
<Router location={location} context={routerContext}>
<DeviceContext.Provider value={{type: deviceType}}>
<AppConfig locals={res.locals}>
<Switch error={error} appState={appState} routes={routes} App={App} />
</AppConfig>
</DeviceContext.Provider>
</Router>
<ServerContext.Provider value={{req, res}}>
<Router location={location} context={routerContext}>
<DeviceContext.Provider value={{type: deviceType}}>
<AppConfig locals={res.locals}>
<Switch error={error} appState={appState} routes={routes} App={App} />
</AppConfig>
</DeviceContext.Provider>
</Router>
</ServerContext.Provider>
)
}

OuterApp.propTypes = {
req: PropTypes.object,
res: PropTypes.object,
error: PropTypes.object,
App: PropTypes.elementType,
Expand Down
4 changes: 3 additions & 1 deletion packages/pwa-kit-react-sdk/src/ssr/universal/components/with-react-query/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {FetchStrategy} from '../fetch-strategy'
import React from 'react'
import {dehydrate, Hydrate, QueryClient, QueryClientProvider} from '@tanstack/react-query'
import ssrPrepass from 'react-ssr-prepass'
import {IsPrePassContext} from '../../contexts'

const isServerSide = typeof window === 'undefined'
const STATE_KEY = '__reactQuery'
Expand All @@ -35,10 +36,11 @@ export const withReactQuery = (Wrapped) => {
/**
* @private
*/
static async doInitAppState({res, appJSX}) {
static async doInitAppState({res, appJSX: _appJSX}) {
const queryClient = (res.locals.__queryClient =
res.locals.__queryClient || new QueryClient())

const appJSX = React.createElement(IsPrePassContext.Provider, {value: true}, _appJSX)
await ssrPrepass(appJSX)

const queryCache = queryClient.getQueryCache()
Expand Down
8 changes: 5 additions & 3 deletions ...t-sdk/src/ssr/universal/device-context.js → ...t-react-sdk/src/ssr/universal/contexts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
/*
* Copyright (c) 2021, salesforce.com, inc.
* Copyright (c) 2022, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import React from 'react'
export default React.createContext()

export const DeviceContext = React.createContext()
export const ServerContext = React.createContext()
export const IsPrePassContext = React.createContext(false)
40 changes: 40 additions & 0 deletions packages/pwa-kit-react-sdk/src/ssr/universal/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2022, Salesforce, Inc.
* All rights reserved.
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import {useContext} from 'react'
import {IsPrePassContext, ServerContext} from './contexts'

/**
* Server context
* @typedef {Object} ServerContext
* @property {Object} req - Request object
* @property {Object} res - Response object
* @property {boolean} isServerSide
*/

/**
* Get the server context
* @returns {ServerContext} ServerContext object
*
* @example
* const {res, isServerSide} = useServerContext()
* if (isServerSide && query.error) { res.status(404) }
*/
export const useServerContext = () => {
const serverContext = useContext(ServerContext)
const isPrePass = useIsPrePass()

return {
...serverContext,
isServerSide: Boolean(serverContext.req) && !isPrePass
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand what you are doing here. But I think this condition is better suited to your old callback implementation. For example, I read this as "isServerSide is true, if I'm on the server, but I'm not prepassing on the server". If this same condition was used with the callback method, then it would read something like "The callback is called only on the server but not during prepass".. It sounds a little easier to understand.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another interesting thing to comment on, is that this hook kind of exposes the notion that we are doing or able to do a prepass on the server. Since the pre-pass logic for the with react query hoc is contained into one file, its probably not ideal to expose that information here. That being said, we are also relying on the context being undefined in the true render to have that conditional logic.

Have you maybe thought about using the res.locals to get a variable before doing the prepass?

For example you could potentially do something like this:

res.locals.isResReadOnly = true
await ssrPrepass(appJSX)
delete res.locals.isResReadOnly

Then in your hook you can access that value and run the callback if you plan on reverting how things currently work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't thought of using res.locals.. will give that a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: after further discussion, we decided to not use res.locals. But clone the given appJSX instead, and make sure that the req/res is undefined for the prepass.

}
}

/**
* @private
*/
export const useIsPrePass = () => useContext(IsPrePassContext)
8 changes: 8 additions & 0 deletions packages/template-typescript-minimal/app/pages/home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {useQuery} from '@tanstack/react-query'
import HelloTS from '../components/hello-typescript'
import HelloJS from '../components/hello-javascript'

import {useServerContext} from 'pwa-kit-react-sdk/ssr/universal/hooks'

interface Props {
value: number
}
Expand Down Expand Up @@ -101,6 +103,12 @@ const Home = ({value}: Props) => {
})
)

const {res, isServerSide} = useServerContext()
if (isServerSide && query.data) {
console.log('--- useServerContext')
res.status(404)
}
bendvc marked this conversation as resolved.
Show resolved Hide resolved

return (
<div>
<style dangerouslySetInnerHTML={{__html: style}} />
Expand Down