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

@W-15907094@ Refactor /mobify paths #1824

Merged
merged 30 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6ebab46
Use dynamic public path for HMR
vcua-mobify Jun 10, 2024
392d9f8
Consolidate /mobify definition
vcua-mobify Jun 10, 2024
1fa3fb7
Remove hard coded mobify path from jest config
vcua-mobify Jun 10, 2024
c293289
Replace /mobify paths in template-retail-react-app
vcua-mobify Jun 11, 2024
f0bc6a8
Move commerce-sdk-react /mobify definitions
vcua-mobify Jun 11, 2024
7c01ae7
Updates to create-app and commerce-sdk-react
vcua-mobify Jun 11, 2024
e7a5564
Fix active-data hook
vcua-mobify Jun 11, 2024
0aeb2e2
Separate public-path entry point from main
vcua-mobify Jun 11, 2024
fc2a9b0
Webpack lint
vcua-mobify Jun 11, 2024
07b4e16
Restore jest config
vcua-mobify Jun 12, 2024
eebe2b1
Merge branch 'develop' into refactor-mobify-paths
vcua-mobify Jun 12, 2024
06505ba
Tweak commerce-sdk-react and add comments
vcua-mobify Jun 13, 2024
43c3a6d
Fix app-config templates
vcua-mobify Jun 13, 2024
91db50e
Add placeholder for /mobify namespace
vcua-mobify Jun 14, 2024
8694c86
Renaming
vcua-mobify Jun 14, 2024
a075a77
Lint
vcua-mobify Jun 14, 2024
5b20384
Remove the branching conditional
vcua-mobify Jun 14, 2024
66ebd4b
Rename ssr-paths
vcua-mobify Jun 14, 2024
88240a6
Namespace for webpack HMR publicPath
vcua-mobify Jun 14, 2024
49c8bd3
Reduce calls to getSLASPrivateProxyPath
vcua-mobify Jun 17, 2024
5459dd1
Merge branch 'develop' into refactor-mobify-paths
vcua-mobify Jun 17, 2024
f90922a
Restore constants
vcua-mobify Jun 18, 2024
039a660
Move webpack public path setter to pwa-kit-dev
vcua-mobify Jun 18, 2024
69b9031
Rename ssr-namespace-paths exports methods
vcua-mobify Jun 20, 2024
8fca554
Update copyright
vcua-mobify Jun 20, 2024
8253e68
Merge branch 'develop' into refactor-mobify-paths
vcua-mobify Jun 21, 2024
a39c65f
Small clean up
vcua-mobify Jun 21, 2024
5b5fd53
Merge branch 'refactor-mobify-paths' of github.com:SalesforceCommerce…
vcua-mobify Jun 21, 2024
849a2b8
Merge branch 'develop' into refactor-mobify-paths
vcua-mobify Jun 21, 2024
bdafdc9
Merge branch 'develop' into refactor-mobify-paths
vcua-mobify Jun 21, 2024
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
8 changes: 5 additions & 3 deletions packages/commerce-sdk-react/src/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {BaseStorage, LocalStorage, CookieStorage, MemoryStorage, StorageType} fr
import {CustomerType} from '../hooks/useCustomerType'
import {getParentOrigin, isOriginTrusted, onClient} from '../utils'
import {
MOBIFY_PATH,
SLAS_PRIVATE_PROXY_PATH,
SLAS_SECRET_WARNING_MSG,
SLAS_SECRET_PLACEHOLDER,
SLAS_SECRET_OVERRIDE_MSG
Expand Down Expand Up @@ -178,9 +180,9 @@ class Auth {
private logger: Logger

constructor(config: AuthConfig) {
// Special endpoint for injecting SLAS private client secret
const baseUrl = config.proxy.split(`/mobify/proxy/api`)[0]
const privateClientEndpoint = `${baseUrl}/mobify/slas/private`
// Special endpoint for injecting SLAS private client secret.
const baseUrl = config.proxy.split(MOBIFY_PATH)[0]
const privateClientEndpoint = `${baseUrl}${SLAS_PRIVATE_PROXY_PATH}`

this.client = new ShopperLogin({
proxy: config.enablePWAKitPrivateClient ? privateClientEndpoint : config.proxy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import {ApiClients} from '../../hooks/types'
import {DEVELOPMENT_ORIGIN, getParentOrigin, isOriginTrusted} from '../../utils'
import {LOCAL_BUNDLE_PATH} from '../../constant'

/** Detects whether the storefront is running in an iframe as part of Storefront Preview.
* @private
Expand All @@ -16,13 +17,15 @@ export const detectStorefrontPreview = () => {
return isOriginTrusted(parentOrigin)
}

/** Returns the URL to load the Storefront Preview client script from the parent origin.
/**
* Returns the URL to load the Storefront Preview client script from the parent origin.
* The client script is served from Runtime Admin and is not a part of the PWA Retail React App bundle.
* @private
*/
export const getClientScript = () => {
const parentOrigin = getParentOrigin() ?? 'https://runtime.commercecloud.com'
return parentOrigin === DEVELOPMENT_ORIGIN
? `${parentOrigin}/mobify/bundle/development/static/storefront-preview.js`
? `${parentOrigin}${LOCAL_BUNDLE_PATH}/static/storefront-preview.js`
: `${parentOrigin}/cc/b2c/preview/preview.client.js`
vcua-mobify marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
6 changes: 6 additions & 0 deletions packages/commerce-sdk-react/src/constant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ export const IFRAME_HOST_ALLOW_LIST = Object.freeze([
'https://runtime-admin-preview.mobify-storefront.com'
])

// We hardcode these here since we don't want commerce-sdk-react to have a dependency on pwa-kit-runtime
export const MOBIFY_PATH = '/mobify'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we previously may have had a discussion about changing the name of this variable. It's 2024 we should definitely not be using "mobify" for any new variables in our code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How are we expecting that this value is configured. I imagine that we would require this to be configurable if it was changed in the rest of the application?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow up discussion happened in Slack convos but to summarize, we will not make the '/mobify' part configurable for now as there are places in MRT that also are looking explicitly for the '/mobify' path

export const PROXY_PATH = `${MOBIFY_PATH}/proxy`
export const LOCAL_BUNDLE_PATH = `${MOBIFY_PATH}/bundle/development`
export const SLAS_PRIVATE_PROXY_PATH = `${MOBIFY_PATH}/slas/private`

export const SLAS_SECRET_WARNING_MSG =
'You are potentially exposing SLAS secret on browser. Make sure to keep it safe and secure!'

Expand Down
5 changes: 3 additions & 2 deletions packages/commerce-sdk-react/src/test-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ import {
} from '@tanstack/react-query'
import nock from 'nock'
import CommerceApiProvider, {CommerceApiProviderProps} from './provider'
import {PROXY_PATH} from './constant'

// Note: this host does NOT exist
// it is intentional b/c we can catch those unintercepted requests
// from log easily. You should always make sure all requests are nocked.
export const DEFAULT_TEST_HOST = 'http://localhost:8888'

export const DEFAULT_TEST_CONFIG = {
proxy: `${DEFAULT_TEST_HOST}/mobify/proxy/api`,
proxy: `${DEFAULT_TEST_HOST}${PROXY_PATH}/api`,
redirectURI: `${DEFAULT_TEST_HOST}/callback`,
clientId: '12345678-1234-1234-1234-123412341234',
organizationId: 'f_ecom_zzrmy_orgf_001',
Expand All @@ -31,7 +32,7 @@ export const DEFAULT_TEST_CONFIG = {
locale: 'en-US',
currency: 'USD',
fetchedToken: 'test-token',
OCAPISessionsURL: `${DEFAULT_TEST_HOST}/mobify/proxy/ocapi/s/RefArch/dw/shop/v22_8/sessions`
OCAPISessionsURL: `${DEFAULT_TEST_HOST}${PROXY_PATH}/ocapi/s/RefArch/dw/shop/v22_8/sessions`
}

export const createQueryClient = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
resolveLocaleFromUrl
} from '@salesforce/retail-react-app/app/utils/site-utils'
import {getConfig} from '@salesforce/pwa-kit-runtime/utils/ssr-config'
import {proxyBasePath} from '@salesforce/pwa-kit-runtime/utils/ssr-namespace-paths'
import {createUrlTemplate} from '@salesforce/retail-react-app/app/utils/url'

import {CommerceApiProvider} from '@salesforce/commerce-sdk-react'
Expand Down Expand Up @@ -55,7 +56,7 @@ const AppConfig = ({children, locals = {}}) => {
redirectURI={`${appOrigin}/callback`}
proxy={`${appOrigin}${commerceApiConfig.proxyPath}`}
headers={headers}
OCAPISessionsURL={`${appOrigin}/mobify/proxy/ocapi/s/${locals.site?.id}/dw/shop/v22_8/sessions`}
OCAPISessionsURL={`${appOrigin}${proxyBasePath}/ocapi/s/${locals.site?.id}/dw/shop/v22_8/sessions`}
{{#if answers.project.commerce.isSlasPrivate}}
// Set 'enablePWAKitPrivateClient' to true use SLAS private client login flows.
// Make sure to also enable useSLASPrivateClient in ssr.js when enabling this setting.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
resolveLocaleFromUrl
} from '@salesforce/retail-react-app/app/utils/site-utils'
import {getConfig} from '@salesforce/pwa-kit-runtime/utils/ssr-config'
import {proxyBasePath} from '@salesforce/pwa-kit-runtime/utils/ssr-namespace-paths'
import {createUrlTemplate} from '@salesforce/retail-react-app/app/utils/url'

import {CommerceApiProvider} from '@salesforce/commerce-sdk-react'
Expand Down Expand Up @@ -55,7 +56,7 @@ const AppConfig = ({children, locals = {}}) => {
redirectURI={`${appOrigin}/callback`}
proxy={`${appOrigin}${commerceApiConfig.proxyPath}`}
headers={headers}
OCAPISessionsURL={`${appOrigin}/mobify/proxy/ocapi/s/${locals.site?.id}/dw/shop/v22_8/sessions`}
OCAPISessionsURL={`${appOrigin}${proxyBasePath}/ocapi/s/${locals.site?.id}/dw/shop/v22_8/sessions`}
{{#if answers.project.commerce.isSlasPrivate}}
// Set 'enablePWAKitPrivateClient' to true use SLAS private client login flows.
// Make sure to also enable useSLASPrivateClient in ssr.js when enabling this setting.
Expand Down
3 changes: 3 additions & 0 deletions packages/pwa-kit-dev/src/configs/jest/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ module.exports = {
DEBUG: true,
NODE_ENV: 'test',
Progressive: {
// BuildOrigin can be any non-empty string. It does not have to be /mobify/xyz
// This is used by tests that call getAssetUrl in pwa-kit-react-sdk to simulate
// asset urls.
buildOrigin: '/mobify/bundle/development/'
}
},
Expand Down
25 changes: 18 additions & 7 deletions packages/pwa-kit-dev/src/configs/webpack/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,18 @@ const getAppEntryPoint = () => {
return resolve('./', EXT_OVERRIDES_DIR_NO_SLASH, 'app', 'main')
}

const getPublicPathEntryPoint = () => {
return resolve(
projectDir,
'node_modules',
'@salesforce',
'pwa-kit-dev',
'ssr',
'server',
'public-path'
)
}

const findDepInStack = (pkg) => {
// Look for the SDK node_modules in two places because in CI,
// pwa-kit-dev is published under a 'dist' directory, which
Expand Down Expand Up @@ -394,7 +406,11 @@ const enableReactRefresh = (config) => {
},
entry: {
...config.entry,
main: ['webpack-hot-middleware/client?path=/__mrt/hmr', getAppEntryPoint()]
main: [
'webpack-hot-middleware/client?path=/__mrt/hmr',
getPublicPathEntryPoint(),
getAppEntryPoint()
]
},
plugins: [
...config.plugins,
Expand All @@ -403,12 +419,7 @@ const enableReactRefresh = (config) => {
new ReactRefreshWebpackPlugin({
overlay: false
})
],
output: {
...config.output,
// Setting this so that *.hot-update.json requests are resolving
publicPath: '/mobify/bundle/development/'
}
Copy link
Contributor Author

@vcua-mobify vcua-mobify Jun 11, 2024

Choose a reason for hiding this comment

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

This is one of the notable changes in this PR. Rather than hard coding /mobify/bundle/development for HMR, the publicPath is now dynamically being set via a new entry point before main.

]
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/pwa-kit-dev/src/ssr/server/build-dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import open from 'open'
import requireFromString from 'require-from-string'
import {RemoteServerFactory} from '@salesforce/pwa-kit-runtime/ssr/server/build-remote-server'
import {proxyConfigs} from '@salesforce/pwa-kit-runtime/utils/ssr-shared'
import {bundleBasePath} from '@salesforce/pwa-kit-runtime/utils/ssr-namespace-paths'
import {
SERVER,
CLIENT,
Expand Down Expand Up @@ -166,7 +167,7 @@ export const DevServerMixin = {
app.__hotServerMiddleware = webpackHotServerMiddleware(app.__compiler)
}

app.use('/mobify/bundle/development', app.__devMiddleware)
app.use(`${bundleBasePath}/development`, app.__devMiddleware)

app.__hmrMiddleware = (_, res) => res.status(501).send('Hot Module Reloading is disabled.')
const clientCompiler = app.__compiler.compilers.find((compiler) => compiler.name === CLIENT)
Expand Down Expand Up @@ -209,7 +210,7 @@ export const DevServerMixin = {
// Proxy bundle asset requests to the local
// build directory.
app.use(
'/mobify/bundle/development',
`${bundleBasePath}/development`,
express.static(path.resolve(process.cwd(), 'src'), {
dotFiles: 'deny',
setHeaders: setLocalAssetHeaders,
Expand Down
20 changes: 20 additions & 0 deletions packages/pwa-kit-dev/src/ssr/server/public-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright (c) 2024, salesforce.com, 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
*/

/**
* This file is used to dynamically set the webpack public path used by HMR via the global webpack variable
* __webpack_public_path__
* See https://webpack.js.org/guides/public-path/
*
* Previously, we hard coded the public path in our webpack config to '/mobify/bundle/development/'
* but we need something more dynamic to support namespaced /mobify paths.
*/

import {bundleBasePath} from '@salesforce/pwa-kit-runtime/utils/ssr-namespace-paths'

/* global __webpack_public_path__: writable */
__webpack_public_path__ = `${bundleBasePath}/development/`
3 changes: 2 additions & 1 deletion packages/pwa-kit-react-sdk/src/ssr/universal/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* @module progressive-web-sdk/ssr/universal/utils
*/
import {proxyConfigs} from '@salesforce/pwa-kit-runtime/utils/ssr-shared'
import {bundleBasePath} from '@salesforce/pwa-kit-runtime/utils/ssr-namespace-paths'

const onClient = typeof window !== 'undefined'

Expand All @@ -22,7 +23,7 @@ export const getAssetUrl = (path) => {
/* istanbul ignore next */
const publicPath = onClient
? `${window.Progressive.buildOrigin}`
: `/mobify/bundle/${process.env.BUNDLE_ID || 'development'}/`
: `${bundleBasePath}/${process.env.BUNDLE_ID || 'development'}/`
return path ? `${publicPath}${path}` : publicPath
}

Expand Down
37 changes: 25 additions & 12 deletions packages/pwa-kit-runtime/src/ssr/server/build-remote-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ import {
X_MOBIFY_QUERYSTRING,
SET_COOKIE,
CACHE_CONTROL,
NO_CACHE,
SLAS_CUSTOM_PROXY_PATH
NO_CACHE
} from './constants'
import {
catchAndLog,
Expand Down Expand Up @@ -40,6 +39,12 @@ import {RESOLVED_PROMISE} from './express'
import http from 'http'
import https from 'https'
import {proxyConfigs, updatePackageMobify} from '../../utils/ssr-shared'
import {
proxyBasePath,
bundleBasePath,
healthCheckPath,
slasPrivateProxyPath
} from '../../utils/ssr-namespace-paths'
import {applyProxyRequestHeaders} from '../../utils/ssr-server/configure-proxy'
import awsServerlessExpress from 'aws-serverless-express'
import expressLogging from 'morgan'
Expand Down Expand Up @@ -205,6 +210,13 @@ export const RemoteServerFactory = {
return true
},

/**
* @private
*/
_isBundleOrProxyPath(url) {
return url.startsWith(proxyBasePath) || url.startsWith(bundleBasePath)
},

/**
* @private
*/
Expand Down Expand Up @@ -457,7 +469,7 @@ export const RemoteServerFactory = {
const processIncomingRequest = (req, res) => {
const options = req.app.options
// If the request is for a proxy or bundle path, do nothing
if (req.originalUrl.startsWith('/mobify/')) {
if (this._isBundleOrProxyPath(req.originalUrl)) {
return
}

Expand Down Expand Up @@ -580,7 +592,7 @@ export const RemoteServerFactory = {
// different types of the 'req' object, and will
// always contain the original full path.
/* istanbul ignore else */
if (!req.originalUrl.startsWith('/mobify/')) {
if (!this._isBundleOrProxyPath(req.originalUrl)) {
req.app.sendMetric(
'RequestTime',
Date.now() - locals.requestStart,
Expand Down Expand Up @@ -626,7 +638,7 @@ export const RemoteServerFactory = {
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
_setupProxying(app, options) {
app.all('/mobify/proxy/*', (_, res) => {
app.all(`${proxyBasePath}/*`, (_, res) => {
return res.status(501).json({
message:
'Environment proxies are not set: https://developer.salesforce.com/docs/commerce/pwa-kit-managed-runtime/guide/proxying-requests.html'
Expand All @@ -638,7 +650,7 @@ export const RemoteServerFactory = {
* @private
*/
_handleMissingSlasPrivateEnvVar(app) {
app.use(SLAS_CUSTOM_PROXY_PATH, (_, res) => {
app.use(slasPrivateProxyPath, (_, res) => {
return res.status(501).json({
message:
'Environment variable PWA_KIT_SLAS_CLIENT_SECRET not set: Please set this environment variable to proceed.'
Expand All @@ -653,28 +665,29 @@ export const RemoteServerFactory = {
if (!options.useSLASPrivateClient) {
return
}
localDevLog(`Proxying ${SLAS_CUSTOM_PROXY_PATH} to ${options.slasTarget}`)

localDevLog(`Proxying ${slasPrivateProxyPath} to ${options.slasTarget}`)

const clientId = options.mobify?.app?.commerceAPI?.parameters?.clientId
const clientSecret = process.env.PWA_KIT_SLAS_CLIENT_SECRET
if (!clientSecret) {
this._handleMissingSlasPrivateEnvVar(app)
this._handleMissingSlasPrivateEnvVar(app, slasPrivateProxyPath)
return
}

const encodedSlasCredentials = Buffer.from(`${clientId}:${clientSecret}`).toString('base64')

app.use(
SLAS_CUSTOM_PROXY_PATH,
slasPrivateProxyPath,
createProxyMiddleware({
target: options.slasTarget,
changeOrigin: true,
pathRewrite: {[SLAS_CUSTOM_PROXY_PATH]: ''},
pathRewrite: {[slasPrivateProxyPath]: ''},
onProxyReq: (proxyRequest, incomingRequest) => {
applyProxyRequestHeaders({
proxyRequest,
incomingRequest,
proxyPath: SLAS_CUSTOM_PROXY_PATH,
proxyPath: slasPrivateProxyPath,
targetHost: options.slasHostName,
targetProtocol: 'https'
})
Expand Down Expand Up @@ -725,7 +738,7 @@ export const RemoteServerFactory = {
* @private
*/
_setupHealthcheck(app) {
app.get('/mobify/ping', (_, res) =>
app.get(`${healthCheckPath}`, (_, res) =>
res.set('cache-control', NO_CACHE).sendStatus(200).end()
)
},
Expand Down
5 changes: 5 additions & 0 deletions packages/pwa-kit-runtime/src/ssr/server/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
export const APPLICATION_OCTET_STREAM = 'application/octet-stream'
export const BUILD = 'build'
export const STATIC_ASSETS = 'static_assets'

/** * @deprecated Use ssr-namespace-paths proxyBasePath instead */
export const PROXY_PATH_PREFIX = '/mobify/proxy'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically a breaking change. Is there a way that we could NOT a breaking change?

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 restored the constants that I removed and annotated them with a @deprecated. Let me know if this works


// All these values MUST be lower case
export const CONTENT_TYPE = 'content-type'
export const CONTENT_ENCODING = 'content-encoding'
Expand All @@ -19,4 +22,6 @@ export const CACHE_CONTROL = 'cache-control'
export const NO_CACHE = 'max-age=0, nocache, nostore, must-revalidate'
export const CONTENT_SECURITY_POLICY = 'content-security-policy'
export const STRICT_TRANSPORT_SECURITY = 'strict-transport-security'

/** * @deprecated Use ssr-namespace-paths.slasPrivateProxyPath instead */
export const SLAS_CUSTOM_PROXY_PATH = '/mobify/slas/private'
Loading
Loading