Skip to content

Commit

Permalink
fix: remove device-context and detect-device-type (#1168)
Browse files Browse the repository at this point in the history
* remove device-context and detect-device-type

* remove ua-parser-js dependency

* Update changelog
  • Loading branch information
joeluong-sfcc authored May 5, 2023
1 parent 3f6d553 commit 9e4f23d
Show file tree
Hide file tree
Showing 13 changed files with 25 additions and 323 deletions.
2 changes: 2 additions & 0 deletions packages/pwa-kit-react-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
## v2.8.0-dev (Mar 03, 2023)
- Remove usage of `device-context` due to deprecation of user agent string. [#1168](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1168)

## v2.7.0 (Mar 03, 2023)
## v2.6.0 (Jan 25, 2023)

Expand Down
19 changes: 8 additions & 11 deletions packages/pwa-kit-react-sdk/src/ssr/browser/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import React, {useRef} from 'react'
import ReactDOM from 'react-dom'
import {BrowserRouter as Router} from 'react-router-dom'
import DeviceContext from '../universal/device-context'
import {ServerContext, CorrelationIdProvider} from '../universal/contexts'
import App from '../universal/components/_app'
import {getAppConfig} from '../universal/compatibility'
Expand Down Expand Up @@ -52,16 +51,14 @@ export const OuterApp = ({routes, error, WrappedApp, locals}) => {
return uuidv4()
}}
>
<DeviceContext.Provider value={{type: window.__DEVICE_TYPE__}}>
<AppConfig locals={locals}>
<Switch
error={error}
appState={window.__PRELOADED_STATE__}
routes={routes}
App={WrappedApp}
/>
</AppConfig>
</DeviceContext.Provider>
<AppConfig locals={locals}>
<Switch
error={error}
appState={window.__PRELOADED_STATE__}
routes={routes}
App={WrappedApp}
/>
</AppConfig>
</CorrelationIdProvider>
</Router>
</ServerContext.Provider>
Expand Down
36 changes: 9 additions & 27 deletions packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ 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 {ServerContext, CorrelationIdProvider} from '../universal/contexts'

import Document from '../universal/components/_document'
Expand All @@ -29,7 +28,7 @@ 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'
import {detectDeviceType, isRemote} from 'pwa-kit-runtime/utils/ssr-server'
import {isRemote} from 'pwa-kit-runtime/utils/ssr-server'
import {proxyConfigs} from 'pwa-kit-runtime/utils/ssr-shared'
import {getConfig} from 'pwa-kit-runtime/utils/ssr-config'
import sprite from 'svg-sprite-loader/runtime/sprite.build'
Expand Down Expand Up @@ -116,7 +115,6 @@ export const render = async (req, res, next) => {
const component = await route.component.getComponent()

// Step 3 - Init the app state
const deviceType = detectDeviceType(req)
const props = {
error: null,
appState: {},
Expand All @@ -125,8 +123,7 @@ export const render = async (req, res, next) => {
res,
App: WrappedApp,
routes,
location,
deviceType
location
}
let appJSX = <OuterApp {...props} />

Expand Down Expand Up @@ -167,8 +164,7 @@ export const render = async (req, res, next) => {
res,
location,
config,
appJSX,
deviceType
appJSX
})
} catch (e) {
// This is an unrecoverable error.
Expand All @@ -192,17 +188,7 @@ export const render = async (req, res, next) => {
}
}

const OuterApp = ({
req,
res,
error,
App,
appState,
routes,
routerContext,
location,
deviceType
}) => {
const OuterApp = ({req, res, error, App, appState, routes, routerContext, location}) => {
const AppConfig = getAppConfig()
return (
<ServerContext.Provider value={{req, res}}>
Expand All @@ -211,11 +197,9 @@ const OuterApp = ({
correlationId={res.locals.requestId}
resetOnPageChange={false}
>
<DeviceContext.Provider value={{type: deviceType}}>
<AppConfig locals={res.locals}>
<Switch error={error} appState={appState} routes={routes} App={App} />
</AppConfig>
</DeviceContext.Provider>
<AppConfig locals={res.locals}>
<Switch error={error} appState={appState} routes={routes} App={App} />
</AppConfig>
</CorrelationIdProvider>
</Router>
</ServerContext.Provider>
Expand All @@ -230,15 +214,14 @@ OuterApp.propTypes = {
appState: PropTypes.object,
routes: PropTypes.array,
routerContext: PropTypes.object,
location: PropTypes.object,
deviceType: PropTypes.string
location: PropTypes.object
}

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

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

const ssrOnly = 'mobify_server_only' in req.query || '__server_only' in req.query
Expand Down Expand Up @@ -300,7 +283,6 @@ const renderApp = (args) => {
const windowGlobals = {
__INITIAL_CORRELATION_ID__: res.locals.requestId,
__CONFIG__: config,
__DEVICE_TYPE__: deviceType,
__PRELOADED_STATE__: appState,
__ERROR__: error,
// `window.Progressive` has a long history at Mobify and some
Expand Down
21 changes: 2 additions & 19 deletions packages/pwa-kit-react-sdk/src/ssr/server/react-rendering.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ const opts = (overrides = {}) => {
}
}

const mobile =
'Mozilla/5.0 (iPhone; U; CPU like Mac OS X; en) AppleWebKit/420+ (KHTML, like Gecko) Version/3.0 Mobile/1A543 Safari/419.3'
const tablet =
'Mozilla/5.0 (iPad; CPU OS 6_1 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Version/6.0 Mobile/10B141 Safari/8536.25'

jest.mock('../universal/compatibility', () => {
const AppConfig = jest.requireActual('../universal/components/_app-config').default
const {withReactQuery} = jest.requireActual('../universal/components/with-react-query')
Expand Down Expand Up @@ -433,28 +428,21 @@ describe('The Node SSR Environment', () => {
console.error(html)
const doc = parse(html)
const include = ['<div>This is a PWA</div>']
const data = dataFromHTML(doc)
const dataScript = doc.querySelectorAll('script[id=mobify-data]')[0]
expect(dataScript.innerHTML.split(/\r\n|\r|\n/)).toHaveLength(1)
expect(data.__DEVICE_TYPE__).toBe('DESKTOP')
include.forEach((s) => expect(html).toEqual(expect.stringContaining(s)))
expect(scriptsAreSafe(doc)).toBe(true)
}
},
{
description: `rendering PWA's for tablet`,
req: {
url: '/pwa/',
headers: {
'User-Agent': tablet
}
url: '/pwa/'
},
assertions: (res) => {
expect(res.statusCode).toBe(200)
const html = res.text
const doc = parse(html)
const data = dataFromHTML(doc)
expect(data.__DEVICE_TYPE__).toBe('TABLET')
const include = ['<div>This is a PWA</div>']
include.forEach((s) => expect(html).toEqual(expect.stringContaining(s)))
expect(scriptsAreSafe(doc)).toBe(true)
Expand All @@ -463,17 +451,12 @@ describe('The Node SSR Environment', () => {
{
description: `rendering PWA's for mobile`,
req: {
url: '/pwa/',
headers: {
'User-Agent': mobile
}
url: '/pwa/'
},
assertions: (res) => {
expect(res.statusCode).toBe(200)
const html = res.text
const doc = parse(html)
const data = dataFromHTML(doc)
expect(data.__DEVICE_TYPE__).toBe('PHONE')
const include = ['<div>This is a PWA</div>']
include.forEach((s) => expect(html).toEqual(expect.stringContaining(s)))
expect(scriptsAreSafe(doc)).toBe(true)
Expand Down

This file was deleted.

2 changes: 2 additions & 0 deletions packages/pwa-kit-runtime/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## v2.8.0-dev (Mar 03, 2023)
- Add optional parameter to override configuration folder used in `getConfig` [#1049](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1049)
- Remove usage of `detect-device-type` due to deprecation of user agent string. [#1168](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1168)

## v2.7.0 (Mar 03, 2023)
- Support Node 16 [#965](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/965)

Expand Down
24 changes: 0 additions & 24 deletions packages/pwa-kit-runtime/package-lock.json

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

1 change: 0 additions & 1 deletion packages/pwa-kit-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
"morgan": "^1.10.0",
"semver": "^7.3.8",
"set-cookie-parser": "^2.6.0",
"ua-parser-js": "^1.0.34",
"whatwg-encoding": "^1.0.5"
},
"devDependencies": {
Expand Down
21 changes: 1 addition & 20 deletions packages/pwa-kit-runtime/src/ssr/server/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ import {
parseCacheControl,
parseEndParameters,
isRemote,
wrapResponseWrite,
detectDeviceType
wrapResponseWrite
} from '../../utils/ssr-server'
import {CONTENT_ENCODING, X_MOBIFY_FROM_CACHE} from './constants'
import {X_MOBIFY_REQUEST_CLASS} from '../../utils/ssr-proxying'
Expand Down Expand Up @@ -53,18 +52,6 @@ export const RESOLVED_PROMISE = Promise.resolve()
* appropriate to include these, the caller should add their values
* (or values based on them) to the options.extras array.
*
* Requests that come to a deployed Express app contain headers that
* identify the device type. By default, this method generates different
* cache keys for different device types (effectively, the values of the
* headers used by getBrowserSize are included in 'options.extras'). To
* suppress this, pass true for options.ignoreDeviceType
* Note: CloudFront will pass 4 device type headers, and ALL of them will
* be present in the request headers, they are 'CloudFront-Is-Desktop-Viewer',
* 'CloudFront-Is-Mobile-Viewer', 'CloudFront-Is-SmartTV-Viewer' and
* 'CloudFront-Is-Tablet-Viewer'. The values can be 'true' or 'false', and it
* is possible that a device falls into more than one device type category
* and multiple device type headers can be 'true' at the same time.
*
* By default, method will generate different cache keys for requests with
* different request classes (effectively, the value of the request-class
* string is included in 'extras'). To suppress this, pass true for
Expand All @@ -74,8 +61,6 @@ export const RESOLVED_PROMISE = Promise.resolve()
* @param [options] {Object} values that affect the cache key generation.
* @param [options.extras] {Array<String|undefined>} extra string values
* to be included in the key.
* @param [options.ignoreDeviceType] {Boolean} set this to true to suppress
* automatic variation of the key by device type.
* @param [options.ignoreRequestClass] {Boolean} set this to true to suppress
* automatic variation of the key by request class.
* @returns {String} the generated key.
Expand All @@ -99,10 +84,6 @@ export const generateCacheKey = (req, options = {}) => {
elements.push(...filteredQueryStrings)
}

if (!options.ignoreDeviceType) {
elements.push(`device=${detectDeviceType(req)}`)
}

if (!options.ignoreRequestClass) {
const requestClass = req.get(X_MOBIFY_REQUEST_CLASS)
if (requestClass) {
Expand Down
46 changes: 0 additions & 46 deletions packages/pwa-kit-runtime/src/ssr/server/express.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -878,52 +878,6 @@ describe('generateCacheKey', () => {
expect(generateCacheKey(mockRequest({url: '/test3?a=2'}))).not.toEqual(result1)
})

test('user agent affects key', () => {
const result1 = generateCacheKey(mockRequest())
const request2 = mockRequest({
headers: {
'user-agent':
'Mozilla/5.0 (iPhone; CPU iPhone OS 11_0 like Mac OS X) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Mobile/15A372 Safari/604.1'
}
})
expect(generateCacheKey(request2)).not.toEqual(result1)
// query string and device type is hashed
expect(generateCacheKey(request2)).toBe(
`/test/${getHashForString(['a=1', 'device=PHONE'].join('-'))}`
)
})

test('CloudFront device headers affect key', () => {
const result1 = generateCacheKey(mockRequest())
const request2 = mockRequest({
headers: {
'CloudFront-Is-Desktop-Viewer': 'false',
'CloudFront-Is-Mobile-Viewer': 'true',
'CloudFront-Is-SmartTV-Viewer': 'false',
'CloudFront-Is-Tablet-Viewer': 'false'
}
})
expect(generateCacheKey(request2)).not.toEqual(result1)
expect(generateCacheKey(request2)).toBe(
`/test/${getHashForString(['a=1', 'device=PHONE'].join('-'))}`
)
})

test('multiple CloudFront device headers affect key', () => {
const request1 = mockRequest({
headers: {
'CloudFront-Is-Desktop-Viewer': 'false',
'CloudFront-Is-Mobile-Viewer': 'true',
'CloudFront-Is-SmartTV-Viewer': 'false',
'CloudFront-Is-Tablet-Viewer': 'true'
}
})

expect(generateCacheKey(request1)).toBe(
`/test/${getHashForString(['a=1', 'device=TABLET'].join('-'))}`
)
})

test('request class affects key', () => {
const result1 = generateCacheKey(mockRequest())
const request2 = mockRequest({
Expand Down
1 change: 0 additions & 1 deletion packages/pwa-kit-runtime/src/utils/ssr-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
// This file is kept for backwards compatibility / simpler imports.
export * from './ssr-server/cached-response'
export * from './ssr-server/configure-proxy'
export * from './ssr-server/detect-device-type'
export * from './ssr-server/metrics-sender'
export * from './ssr-server/outgoing-request-hook'
export * from './ssr-server/parse-end-parameters'
Expand Down
Loading

0 comments on commit 9e4f23d

Please sign in to comment.