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.

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

Integrate Server-Side ReactQuery Support (Non-Breaking Change) #724

Merged
merged 20 commits into from
Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
9,538 changes: 31 additions & 9,507 deletions package-lock.json

Large diffs are not rendered by default.

1,704 changes: 10 additions & 1,694 deletions packages/commerce-sdk-react/package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions packages/pwa-kit-dev/src/configs/webpack/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ const baseConfig = (target) => {
extensions: ['.ts', '.tsx', '.js', '.jsx', '.json'],
alias: {
'babel-runtime': findInProjectThenSDK('babel-runtime'),
'@tanstack/react-query': findInProjectThenSDK('@tanstack/react-query'),
'@loadable/component': findInProjectThenSDK('@loadable/component'),
'@loadable/server': findInProjectThenSDK('@loadable/server'),
'@loadable/webpack-plugin': findInProjectThenSDK(
Expand Down
24 changes: 24 additions & 0 deletions packages/pwa-kit-react-sdk/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions packages/pwa-kit-react-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"@loadable/babel-plugin": "^5.13.2",
"@loadable/server": "^5.15.0",
"@loadable/webpack-plugin": "^5.15.0",
"@tanstack/react-query": "^4.0.10",
Copy link
Collaborator

@bendvc bendvc Sep 19, 2022

Choose a reason for hiding this comment

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

It doesn't look like we can enforce the version of react-query that is used in the template code without making a breaking change. That change being making react-query a peer dependency.

We've used optional peer dependencies in other modules, but optional peer deps aren't supported in some of the lower versions of npm that we support.

Do you think it's ok to just leave this as is, and if someone was to upgrade the version of their react-query in their project we simply make no promises on it working with the SDK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, I'd use the optional peer dep. We've used it before. I think it's just ignored if it's not supported by an NPM version, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So for node 7 which doesn't know anything about peerDependenciesMeta optional, if would just think that react-query is a peer dep. Which is a breaking change.

"cross-env": "^5.2.0",
"event-emitter": "^0.3.5",
"glob": "7.1.1",
Expand All @@ -51,6 +52,7 @@
"mkdirp": "^1.0.4",
"prop-types": "^15.6.0",
"pwa-kit-runtime": "^2.3.0-dev",
"react-ssr-prepass": "^1.5.0",
"react-uid": "^2.2.0",
"serialize-javascript": "^6.0.0",
"svg-sprite-loader": "^6.0.11",
Expand Down
3 changes: 2 additions & 1 deletion packages/pwa-kit-react-sdk/src/ssr/browser/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import ReactDOM from 'react-dom'
import {BrowserRouter as Router} from 'react-router-dom'
import DeviceContext from '../universal/device-context'
import App from '../universal/components/_app'
import AppConfig from '../universal/components/_app-config'
import {getAppConfig} from '../universal/compatibility'
import Switch from '../universal/components/switch'
import {getRoutes, routeComponent} from '../universal/components/route-component'
import {loadableReady} from '@loadable/component'
Expand All @@ -34,6 +34,7 @@ export const registerServiceWorker = (url) => {

/* istanbul ignore next */
export const start = () => {
const AppConfig = getAppConfig()
const rootEl = document.getElementsByClassName('react-target')[0]
const data = JSON.parse(document.getElementById('mobify-data').innerHTML)

Expand Down
144 changes: 65 additions & 79 deletions packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import Document from '../universal/components/_document'
import App from '../universal/components/_app'
import Throw404 from '../universal/components/throw-404'

import AppConfig from '../universal/components/_app-config'
import {getAppConfig} from '../universal/compatibility'
import Switch from '../universal/components/switch'
import {getRoutes, routeComponent} from '../universal/components/route-component'
import * as errors from '../universal/errors'
Expand Down Expand Up @@ -72,52 +72,6 @@ const logAndFormatError = (err) => {
}
}

const initAppState = async ({App, component, match, route, req, res, location}) => {
if (component === Throw404) {
// Don't init if there was no match
return {
error: new errors.HTTPNotFound('Not found'),
appState: {}
}
}

const {params} = match

const components = [App, route.component]
const promises = components.map((c) =>
c.getProps
? c.getProps({
req,
res,
params,
location
})
: Promise.resolve({})
)
let returnVal = {}

try {
const [appProps, pageProps] = await Promise.all(promises)
const appState = {
appProps,
pageProps,
__STATE_MANAGEMENT_LIBRARY: AppConfig.freeze(res.locals)
}

returnVal = {
error: undefined,
appState: appState
}
} catch (error) {
returnVal = {
error: error || new Error(),
appState: {}
}
}

return returnVal
}

/**
* This is the main react-rendering function for SSR. It is an Express handler.
*
Expand All @@ -129,11 +83,10 @@ const initAppState = async ({App, component, match, route, req, res, location})
* @return {Promise}
*/
export const render = async (req, res, next) => {
const AppConfig = getAppConfig()
// Get the application config which should have been stored at this point.
const config = getConfig()

// AppConfig.restore *must* come before using getRoutes() or routeComponent()
// to inject arguments into the wrapped component's getProps methods.
AppConfig.restore(res.locals)

const routes = getRoutes(res.locals)
Expand Down Expand Up @@ -162,30 +115,60 @@ export const render = async (req, res, next) => {
const component = await route.component.getComponent()

// Step 3 - Init the app state
const {appState, error: appStateError} = await initAppState({
App: WrappedApp,
component,
match,
route,
const deviceType = detectDeviceType(req)
const props = {
error: null,
appState: {},
routerContext: {},
req,
res,
location
})

// Step 4 - Render the App
let renderResult
const args = {
App: WrappedApp,
appState,
appStateError: appStateError && logAndFormatError(appStateError),
routes,
req,
res,
location,
config
deviceType
}
let appJSX = <OuterApp {...props} />

let appState, appStateError

if(component === Throw404) {
appState = {}
appStateError = new errors.HTTPNotFound('Not found')
} else {
const ret = await AppConfig.initAppState({
App: WrappedApp,
component,
match,
route,
req,
res,
location,
appJSX
})
appState = {
...ret.appState,
__STATE_MANAGEMENT_LIBRARY: AppConfig.freeze(res.locals)
}
appStateError = ret.error
}

appJSX = React.cloneElement(appJSX, {error: appStateError, appState})

// Step 4 - Render the App
let renderResult
try {
renderResult = renderApp(args)
renderResult = renderApp({
App: WrappedApp,
appState,
appStateError: appStateError && logAndFormatError(appStateError),
routes,
req,
res,
location,
config,
appJSX,
deviceType
})
} catch (e) {
// This is an unrecoverable error.
// (errors handled by the AppErrorBoundary are considered recoverable)
Expand All @@ -208,10 +191,9 @@ export const render = async (req, res, next) => {
}
}

const renderAppHtml = (req, res, error, appData) => {
const {App, appState, routes, routerContext, location, extractor, deviceType} = appData

let appJSX = (
const OuterApp = ({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}>
Expand All @@ -220,33 +202,37 @@ const renderAppHtml = (req, res, error, appData) => {
</DeviceContext.Provider>
</Router>
)

appJSX = extractor.collectChunks(appJSX)
return ReactDOMServer.renderToString(appJSX)
}

const renderToString = (jsx, extractor) =>
ReactDOMServer.renderToString(extractor.collectChunks(jsx))

const renderApp = (args) => {
const {req, res, appStateError, App, appState, location, routes, config} = args
const deviceType = detectDeviceType(req)
const {req, res, appStateError, appJSX, appState, config, deviceType} = args
const extractor = new ChunkExtractor({statsFile: BUNDLES_PATH, publicPath: getAssetUrl()})
const routerContext = {}
const appData = {App, appState, location, routes, routerContext, deviceType, extractor}

const ssrOnly = 'mobify_server_only' in req.query || '__server_only' in req.query
const prettyPrint = 'mobify_pretty' in req.query || '__pretty_print' in req.query
const indent = prettyPrint ? 8 : 0

let routerContext
let appHtml
let renderError
// It's important that we render the App before extracting the script elements,
// otherwise it won't return the correct chunks.

try {
appHtml = renderAppHtml(req, res, appStateError, appData)
routerContext = {}
appHtml = renderToString(React.cloneElement(appJSX, {routerContext}), extractor)
} catch (e) {
// This will catch errors thrown from the app and pass the error
// to the AppErrorBoundary component, and renders the error page.
routerContext = {}
renderError = logAndFormatError(e)
appHtml = renderAppHtml(req, res, renderError, appData)
appHtml = renderToString(
React.cloneElement(appJSX, {routerContext, error: renderError}),
extractor
)
}

// Setting type: 'application/json' stops the browser from executing the code.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import request from 'supertest'
import {parse} from 'node-html-parser'
import path from 'path'
import {isRemote} from 'pwa-kit-runtime/utils/ssr-server'
import AppConfig from '../universal/components/_app-config'
import {getAppConfig} from '../universal/compatibility'

const opts = (overrides = {}) => {
const fixtures = path.join(__dirname, '..', '..', 'ssr', 'server', 'test_fixtures')
Expand Down Expand Up @@ -605,6 +605,7 @@ describe('The Node SSR Environment', () => {
description: `AppConfig errors are caught`,
req: {url: '/pwa/'},
mocks: () => {
const AppConfig = getAppConfig()
jest.spyOn(AppConfig.prototype, 'render').mockImplementation(() => {
throw new Error()
})
Expand Down
17 changes: 17 additions & 0 deletions packages/pwa-kit-react-sdk/src/ssr/universal/compatibility.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import AppConfig from './components/_app-config'
import {withLegacyGetProps} from './withLegacyGetProps'

let _appConfig = AppConfig

/**
* If a user hasn't opted into a fetchStrategy, automatically
* opt them into getProps – this maintains backward compatibility.
*
* @private
*/
export const getAppConfig = () => {
if (!_appConfig.initAppState) {
_appConfig = withLegacyGetProps(_appConfig)
}
return _appConfig
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {withRouter} from 'react-router-dom'
import hoistNonReactStatic from 'hoist-non-react-statics'
import {AppErrorContext} from '../../components/app-error-boundary'
import Throw404 from '../../components/throw-404'
import AppConfig from '../../components/_app-config'
import {getAppConfig} from '../../compatibility'
import routes from '../../routes'
import {pages as pageEvents} from '../../events'

Expand Down Expand Up @@ -54,6 +54,7 @@ const withErrorHandling = (Wrapped) => {
* that can be used to fetch data on the server and on the client, seamlessly.
*/
export const routeComponent = (Wrapped, isPage, locals) => {
const AppConfig = getAppConfig()
bendvc marked this conversation as resolved.
Show resolved Hide resolved
const extraArgs = AppConfig.extraGetPropsArgs(locals)

/* istanbul ignore next */
Expand Down
22 changes: 22 additions & 0 deletions packages/pwa-kit-react-sdk/src/ssr/universal/fetchStrategy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import React from 'react'

export class FetchStrategy extends React.Component {
render() {
return <div />
}

static async initAppState(args) {
try {
const promises = this.getInitializers().map((fn) => fn(args))
return {
error: undefined,
appState: Object.assign({}, ...(await Promise.all(promises)))
}
} catch (error) {
return {
error: error || new Error(),
appState: {}
}
}
}
}
Loading