Skip to content

Commit

Permalink
Optimize Server-side performance (#667)
Browse files Browse the repository at this point in the history
* Use LocaleContext and SiteContext

* Declare AppConfigProvider

* Optimization use `AppConfig` context & `urlTemplateLiteral` without loops

* Fix missing forward slash on `urlTemplateLiteral`

* Clean up

* Remove `buildPathWithUrlConfig()` & Start fixing test

* Fix some tests

* Clean up

* Combine default cases

* Update url.js

* Refactor switch

* Add docs

* Minor naming

* Update packages/template-retail-react-app/app/utils/url.js

Co-authored-by: Will Harney <[email protected]>

* PR Feedback rename c to config

* Update packages/template-retail-react-app/app/components/link/index.jsx

Co-authored-by: Will Harney <[email protected]>

* PR Feedback naming

* Update `getPathWithLocale` signature & Clean up

* Fix tests

* Fix more tests

* Small refactor using site and locale references in `createUrlTemplate`

* LocaleProvider use locale id

* Fix disallowParams test

* Footer Locale Select using the right params

* Add `createUrlTemplate` tests

* Small tweaks to `createUrlTemplate` tests

* Run all tests on link component

* Fix link tests

* Small naming

* Remove `homeUrlBuilder`

* Run all url tests

* Extend `createUrlTemplate` to replace `homeUrlBuilder`

* URL templateConfig not needed

* Adjust `createUrlTemplate` home extension

* PR Feedback

* Test `useUrlTemplate` Vincent M idea

* lint

* Move SiteProvider and LocaleProvider to AppConfig

* Refactor Site, Locale and UrlTemplate Providers into `MultiSiteProvider`

* Rename `useUrlTemplate` hook to `useMultiSite` hook

* PR Feedback use `useCallback` in `useMultiSite` hook

* Rename `fillUrlTemplate` to `buildUrl`

Co-authored-by: Will Harney <[email protected]>
  • Loading branch information
adamraya and wjhsf authored Aug 4, 2022
1 parent ce38ea5 commit f3951c3
Show file tree
Hide file tree
Showing 33 changed files with 600 additions and 644 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import {
CustomerProductListsProvider,
CustomerProvider
} from '../../commerce-api/contexts'
import {MultiSiteProvider} from '../../contexts'
import {resolveSiteFromUrl} from '../../utils/site-utils'
import {resolveLocaleFromUrl} from '../../utils/utils'
import {getConfig} from 'pwa-kit-runtime/utils/ssr-config'
import {createUrlTemplate} from '../../utils/url'

/**
* Use the AppConfig component to inject extra arguments into the getProps
Expand All @@ -36,15 +38,17 @@ const AppConfig = ({children, locals = {}}) => {
const [customer, setCustomer] = useState(null)

return (
<CommerceAPIProvider value={locals.api}>
<CustomerProvider value={{customer, setCustomer}}>
<BasketProvider value={{basket, setBasket}}>
<CustomerProductListsProvider>
<ChakraProvider theme={theme}>{children}</ChakraProvider>
</CustomerProductListsProvider>
</BasketProvider>
</CustomerProvider>
</CommerceAPIProvider>
<MultiSiteProvider site={locals.site} locale={locals.locale} buildUrl={locals.buildUrl}>
<CommerceAPIProvider value={locals.api}>
<CustomerProvider value={{customer, setCustomer}}>
<BasketProvider value={{basket, setBasket}}>
<CustomerProductListsProvider>
<ChakraProvider theme={theme}>{children}</ChakraProvider>
</CustomerProductListsProvider>
</BasketProvider>
</CustomerProvider>
</CommerceAPIProvider>
</MultiSiteProvider>
)
}

Expand All @@ -66,13 +70,19 @@ AppConfig.restore = (locals = {}) => {
apiConfig.parameters.siteId = site.id

locals.api = new CommerceAPI({...apiConfig, locale: locale.id, currency})
locals.buildUrl = createUrlTemplate(appConfig, site.alias || site.id, locale.id)
locals.site = site
locals.locale = locale.id
}

AppConfig.freeze = () => undefined

AppConfig.extraGetPropsArgs = (locals = {}) => {
return {
api: locals.api
api: locals.api,
buildUrl: locals.buildUrl,
site: locals.site,
locale: locals.locale
}
}

Expand Down
36 changes: 14 additions & 22 deletions packages/template-retail-react-app/app/components/_app/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,19 @@ import useShopper from '../../commerce-api/hooks/useShopper'
import useCustomer from '../../commerce-api/hooks/useCustomer'
import {AuthModal, useAuthModal} from '../../hooks/use-auth-modal'
import {AddToCartModalProvider} from '../../hooks/use-add-to-cart-modal'
import useSite from '../../hooks/use-site'
import useLocale from '../../hooks/use-locale'
import useWishlist from '../../hooks/use-wishlist'

// Localization
import {IntlProvider} from 'react-intl'

// Others
import {watchOnlineStatus, flatten} from '../../utils/utils'
import {homeUrlBuilder, getPathWithLocale, buildPathWithUrlConfig} from '../../utils/url'
import {getTargetLocale, fetchTranslations} from '../../utils/locale'
import {DEFAULT_SITE_TITLE, HOME_HREF, THEME_COLOR} from '../../constants'

import Seo from '../seo'
import {resolveSiteFromUrl} from '../../utils/site-utils'
import useMultiSite from '../../hooks/use-multi-site'

const DEFAULT_NAV_DEPTH = 3
const DEFAULT_ROOT_CATEGORY = 'root'
Expand All @@ -63,18 +61,11 @@ const App = (props) => {
const location = useLocation()
const authModal = useAuthModal()
const customer = useCustomer()

const site = useSite()
const locale = useLocale()
const {site, locale, buildUrl} = useMultiSite()

const [isOnline, setIsOnline] = useState(true)
const styles = useStyleConfig('App')

const configValues = {
locale: locale.alias || locale.id,
site: site.alias || site.id
}

const {isOpen, onOpen, onClose} = useDisclosure()

// Used to conditionally render header/footer for checkout page
Expand Down Expand Up @@ -115,15 +106,16 @@ const App = (props) => {

const onLogoClick = () => {
// Goto the home page.
const path = homeUrlBuilder(HOME_HREF, {locale, site})
const path = buildUrl(HOME_HREF)

history.push(path)

// Close the drawer.
onClose()
}

const onCartClick = () => {
const path = buildPathWithUrlConfig('/cart', configValues)
const path = buildUrl('/cart')
history.push(path)

// Close the drawer.
Expand All @@ -133,7 +125,7 @@ const App = (props) => {
const onAccountClick = () => {
// Link to account page for registered customer, open auth modal otherwise
if (customer.isRegistered) {
const path = buildPathWithUrlConfig('/account', configValues)
const path = buildUrl('/account')
history.push(path)
} else {
// if they already are at the login page, do not show login modal
Expand All @@ -143,7 +135,7 @@ const App = (props) => {
}

const onWishlistClick = () => {
const path = buildPathWithUrlConfig('/account/wishlist', configValues)
const path = buildUrl('/account/wishlist')
history.push(path)
}

Expand Down Expand Up @@ -184,19 +176,15 @@ const App = (props) => {
<link
rel="alternate"
hrefLang={locale.id.toLowerCase()}
href={`${appOrigin}${getPathWithLocale(locale.id, {
location
})}`}
href={`${appOrigin}${buildUrl(location.pathname)}`}
key={locale.id}
/>
))}
{/* A general locale as fallback. For example: "en" if default locale is "en-GB" */}
<link
rel="alternate"
hrefLang={site.l10n.defaultLocale.slice(0, 2)}
href={`${appOrigin}${getPathWithLocale(site.l10n.defaultLocale, {
location
})}`}
href={`${appOrigin}${buildUrl(location.pathname)}`}
/>
{/* A wider fallback for user locales that the app does not support */}
<link rel="alternate" hrefLang="x-default" href={`${appOrigin}/`} />
Expand All @@ -222,11 +210,15 @@ const App = (props) => {
onClose={onClose}
onLogoClick={onLogoClick}
root={allCategories[DEFAULT_ROOT_CATEGORY]}
locale={locale}
/>
</HideOnDesktop>

<HideOnMobile>
<ListMenu root={allCategories[DEFAULT_ROOT_CATEGORY]} />
<ListMenu
root={allCategories[DEFAULT_ROOT_CATEGORY]}
locale={locale}
/>
</HideOnMobile>
</Header>
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ import {Helmet} from 'react-helmet'
import App from './index.jsx'
import {renderWithProviders} from '../../utils/test-utils'
import {DEFAULT_LOCALE} from '../../utils/test-utils'
import useSite from '../../hooks/use-site'
import useMultiSite from '../../hooks/use-multi-site'
import messages from '../../translations/compiled/en-GB.json'
import mockConfig from '../../../config/mocks/default'
jest.mock('../../hooks/use-site', () => jest.fn())
jest.mock('../../hooks/use-multi-site', () => jest.fn())
let windowSpy
beforeAll(() => {
jest.spyOn(console, 'log').mockImplementation(jest.fn())
Expand All @@ -40,8 +40,21 @@ describe('App', () => {
...mockConfig.app.sites[0],
alias: 'uk'
}

const locale = DEFAULT_LOCALE

const buildUrl = jest.fn().mockImplementation((href, site, locale) => {
return `${site ? `/${site}` : ''}${locale ? `/${locale}` : ''}${href}`
})

const resultUseMultiSite = {
site,
locale,
buildUrl
}

test('App component is rendered appropriately', () => {
useSite.mockImplementation(() => site)
useMultiSite.mockImplementation(() => resultUseMultiSite)
renderWithProviders(
<App targetLocale={DEFAULT_LOCALE} defaultLocale={DEFAULT_LOCALE} messages={messages}>
<p>Any children here</p>
Expand All @@ -66,7 +79,7 @@ describe('App', () => {
})

test('The localized hreflang links exist in the html head', () => {
useSite.mockImplementation(() => site)
useMultiSite.mockImplementation(() => resultUseMultiSite)
renderWithProviders(
<App targetLocale={DEFAULT_LOCALE} defaultLocale={DEFAULT_LOCALE} messages={messages} />
)
Expand All @@ -77,7 +90,7 @@ describe('App', () => {
const hasGeneralLocale = ({hrefLang}) => hrefLang === DEFAULT_LOCALE.slice(0, 2)

// `length + 2` because one for a general locale and the other with x-default value
expect(hreflangLinks.length).toBe(site.l10n.supportedLocales.length + 2)
expect(hreflangLinks.length).toBe(resultUseMultiSite.site.l10n.supportedLocales.length + 2)

expect(hreflangLinks.some((link) => hasGeneralLocale(link))).toBe(true)
expect(hreflangLinks.some((link) => link.hrefLang === 'x-default')).toBe(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import useCustomer from '../../commerce-api/hooks/useCustomer'
import LoadingSpinner from '../loading-spinner'

import useNavigation from '../../hooks/use-navigation'
import useSite from '../../hooks/use-site'
import useMultiSite from '../../hooks/use-multi-site'

// The FONT_SIZES and FONT_WEIGHTS constants are used to control the styling for
// the accordion buttons as their current depth. In the below definition we assign
Expand Down Expand Up @@ -84,7 +84,7 @@ const DrawerMenu = ({isOpen, onClose = noop, onLogoClick = noop, root}) => {
const styles = useMultiStyleConfig('DrawerMenu')
const drawerSize = useBreakpointValue({sm: PHONE_DRAWER_SIZE, md: TABLET_DRAWER_SIZE})
const socialIconVariant = useBreakpointValue({base: 'flex', md: 'flex-start'})
const site = useSite()
const {site, buildUrl} = useMultiSite()
const {l10n} = site
const [showLoading, setShowLoading] = useState(false)
const onSignoutClick = async () => {
Expand Down Expand Up @@ -272,7 +272,7 @@ const DrawerMenu = ({isOpen, onClose = noop, onLogoClick = noop, root}) => {
locales={supportedLocaleIds}
onSelect={(newLocale) => {
// Update the `locale` in the URL.
const newUrl = getPathWithLocale(newLocale, {
const newUrl = getPathWithLocale(newLocale, buildUrl, {
disallowParams: ['refine']
})
window.location = newUrl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@ import SocialIcons from '../social-icons'
import {HideOnDesktop, HideOnMobile} from '../responsive'
import {getPathWithLocale} from '../../utils/url'
import LocaleText from '../locale-text'
import useSite from '../../hooks/use-site'
import useMultiSite from '../../hooks/use-multi-site'

const Footer = ({...otherProps}) => {
const styles = useMultiStyleConfig('Footer')
const intl = useIntl()
const [locale, setLocale] = useState(intl.locale)
const site = useSite()
const {site, buildUrl} = useMultiSite()
const {l10n} = site

const supportedLocaleIds = l10n?.supportedLocales.map((locale) => locale.id)
const showLocaleSelector = supportedLocaleIds?.length > 1

Expand Down Expand Up @@ -136,7 +137,7 @@ const Footer = ({...otherProps}) => {
setLocale(target.value)

// Update the `locale` in the URL.
const newUrl = getPathWithLocale(target.value, {
const newUrl = getPathWithLocale(target.value, buildUrl, {
disallowParams: ['refine']
})

Expand Down
14 changes: 3 additions & 11 deletions packages/template-retail-react-app/app/components/link/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,12 @@ import React from 'react'
import PropTypes from 'prop-types'
import {Link as ChakraLink} from '@chakra-ui/react'
import {Link as SPALink, NavLink as NavSPALink} from 'react-router-dom'
import {buildPathWithUrlConfig} from '../../utils/url'
import useSite from '../../hooks/use-site'
import useLocale from '../../hooks/use-locale'
import useMultiSite from '../../hooks/use-multi-site'

const Link = React.forwardRef(({href, to, useNavLink = false, ...props}, ref) => {
const _href = to || href
const site = useSite()
const locale = useLocale()

// if alias is not defined, use site id
const updatedHref = buildPathWithUrlConfig(_href, {
locale: locale.alias || locale.id,
site: site.alias || site.id
})
const {buildUrl} = useMultiSite()
const updatedHref = buildUrl(_href)
return (
<ChakraLink
as={useNavLink ? NavSPALink : SPALink}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ test('renders a link with locale prepended', () => {
delete window.location
window.location = new URL('/us/en-US', 'https://www.example.com')
const {getByText} = renderWithProviders(<Link href="/mypage">My Page</Link>, {
wrapperProps: {locale: 'en-US'}
wrapperProps: {locale: 'en-US', siteAlias: 'us', appConfig: mockConfig.app}
})
expect(getByText(/My Page/i)).toHaveAttribute('href', '/us/en-US/mypage')
})
Expand All @@ -49,7 +49,7 @@ test('renders a link with locale and site as query param', () => {
delete window.location
window.location = new URL('https://www.example.com/women/dresses?site=us&locale=en-US')
const {getByText} = renderWithProviders(<Link href="/mypage">My Page</Link>, {
wrapperProps: {locale: 'en-US'}
wrapperProps: {locale: 'en-US', siteAlias: 'us', appConfig: newConfig.app}
})

expect(getByText(/My Page/i)).toHaveAttribute('href', `/mypage?site=us&locale=en-US`)
Expand All @@ -60,7 +60,7 @@ test('accepts `to` prop as well', () => {
delete window.location
window.location = new URL('us/en-US', 'https://www.example.com')
const {getByText} = renderWithProviders(<Link to="/mypage">My Page</Link>, {
wrapperProps: {locale: 'en-US'}
wrapperProps: {locale: 'en-US', siteAlias: 'us', appConfig: mockConfig.app}
})
expect(getByText(/My Page/i)).toHaveAttribute('href', '/us/en-US/mypage')
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import SearchInput from './index'
import Suggestions from './partials/suggestions'
import {noop} from '../../utils/utils'
import mockSearchResults from '../../commerce-api/mocks/searchResults'
import mockConfig from '../../../config/mocks/default'

jest.mock('../../commerce-api/utils', () => {
const originalModule = jest.requireActual('../../commerce-api/utils')
Expand Down Expand Up @@ -53,7 +54,9 @@ test('renders Popover if recent searches populated', async () => {
})

test('changes url when enter is pressed', async () => {
renderWithProviders(<SearchInput />)
renderWithProviders(<SearchInput />, {
wrapperProps: {siteAlias: 'uk', appConfig: mockConfig.app}
})
const searchInput = document.querySelector('input[type="search"]')
await user.type(searchInput, 'Dresses{enter}')
await waitFor(() => expect(window.location.pathname).toEqual(createPathWithDefaults('/search')))
Expand Down
Loading

0 comments on commit f3951c3

Please sign in to comment.