Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

feat(conditionalIntl): added env var for Itnl #1258

Merged
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
cdfc459
feat(conditionalIntl): added env var for Itnl
timBarton96 Jan 25, 2024
7ef00ed
feat(conditionalIntl): fixed nonce
timBarton96 Jan 25, 2024
4a5db19
Merge branch 'main' into feat/conditionalIntlPolyfill
Matthew-Mallimo Jan 29, 2024
d76bbe5
feat(conditionalIntl): renamed env var, conditional I18n script
timBarton96 Feb 1, 2024
edb789a
feat(conditionalItnl): renderI18nScript return early
timBarton96 Feb 2, 2024
c3265dc
feat(conditionalIntl): renaming tests for env var script
timBarton96 Feb 2, 2024
a91730d
Merge branch 'main' into feat/conditionalIntlPolyfill
Matthew-Mallimo Feb 5, 2024
cc1706f
Merge branch 'main' into feat/conditionalIntlPolyfill
JAdshead Feb 5, 2024
43f9921
Merge branch 'main' into feat/conditionalIntlPolyfill
Matthew-Mallimo Feb 14, 2024
5c0aab4
feat(conditionalIntl): disable getLocalePack with env var
timBarton96 Feb 19, 2024
ce17ecb
Merge branch 'main' into feat/conditionalIntlPolyfill
Matthew-Mallimo Feb 20, 2024
74b64c2
Merge branch 'main' into feat/conditionalIntlPolyfill
Matthew-Mallimo Mar 6, 2024
2ceae45
feat(conditionalIntl): bump one-app-ducks
timBarton96 Mar 6, 2024
65a6992
feat(conditionalIntl): fix bullet in doc
timBarton96 Mar 6, 2024
617ec89
feat(conditionalPolyfill): uncomment logging
timBarton96 Mar 8, 2024
d89caab
Merge branch 'main' into feat/conditionalIntlPolyfill
10xLaCroixDrinker Mar 12, 2024
ca24808
chore(ducks): take latest ducks
code-forger Mar 26, 2024
a5a231b
Merge branch 'main' into feat/conditionalIntlPolyfill
code-forger Mar 26, 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
11 changes: 11 additions & 0 deletions __tests__/client/prerender.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ describe('loadPrerenderScripts', () => {
expect(getLocalePack).not.toHaveBeenCalled();
})
);

it('should still resolve if useNativeIntl is set to true but not call getLocalePack', async () => {
window.useNativeIntl = true;
const { getLocalePack } = require('@americanexpress/one-app-ducks');
const initialState = fromJS({ intl: { activeLocale: 'es-ES' } });
getLocalePack.mockClear();

await loadPrerenderScripts(initialState);

expect(getLocalePack).not.toHaveBeenCalled();
});
});

describe('moveHelmetScripts', () => {
Expand Down
17 changes: 17 additions & 0 deletions __tests__/server/config/env/runTime.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ describe('runTime', () => {
'ONE_CLIENT_ROOT_MODULE_NAME',
'ONE_ENABLE_POST_TO_MODULE_ROUTES',
'ONE_MAX_POST_REQUEST_PAYLOAD',
'ONE_CONFIG_USE_NATIVE_INTL',
].forEach((name) => {
origEnvVarVals[name] = process.env[name];
});
Expand Down Expand Up @@ -547,4 +548,20 @@ describe('runTime', () => {
expect(otelServiceName.defaultValue).toBe('One App');
});
});

describe('ONE_CONFIG_USE_NATIVE_INTL', () => {
const useNativeIntl = getEnvVarConfig('ONE_CONFIG_USE_NATIVE_INTL');

it('should have a default value of false', () => {
expect(useNativeIntl.defaultValue).toBe('false');
});

it('should normalise the value to false when not explicitly true', () => {
expect(useNativeIntl.normalize('Value')).toBe('false');
expect(useNativeIntl.normalize('VALUE')).toBe('false');
expect(useNativeIntl.normalize('true')).toBe('true');
expect(useNativeIntl.normalize('TRUE')).toBe('true');
expect(useNativeIntl.normalize('FALSE')).toBe('false');
});
});
});
31 changes: 31 additions & 0 deletions __tests__/server/plugins/reactHtml/index.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import reactHtml, {
renderModuleScripts,
renderExternalFallbacks,
checkStateForRedirectAndStatusCode,
renderEnvironmentVariables,
} from '../../../../src/server/plugins/reactHtml';
// _client is a method to control the mock
// eslint-disable-next-line import/named
Expand Down Expand Up @@ -283,6 +284,8 @@ describe('reactHtml', () => {
cdnUrl: '/cdnUrl/',
rootModuleName: 'test-root',
}));

process.env.ONE_CONFIG_USE_NATIVE_INTL = 'false';
});

function removeInitialState(body) {
Expand Down Expand Up @@ -430,6 +433,15 @@ describe('reactHtml', () => {
expect(reply.send.mock.calls[0][0]).not.toContain('src="/cdnUrl/app/1.2.3-rc.4-abc123/i18n/');
});

it('sends a rendered page without the locale data script tag when te ONE_CONFIG_USE_NATIVE_INTL environment variable is true', () => {
process.env.ONE_CONFIG_USE_NATIVE_INTL = 'true';
setFullMap();
sendHtml(request, reply);

expect(reply.send).toHaveBeenCalledTimes(1);
expect(reply.send.mock.calls[0][0]).not.toContain('src="/cdnUrl/app/1.2.3-rc.4-abc123/i18n/');
});

it('sends a rendered page with the module styles and scripts', () => {
sendHtml(request, reply);
expect(reply.send).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -1262,4 +1274,23 @@ describe('reactHtml', () => {
expect(reply.send.mock.calls[0][0]).toContain('<title>One App</title>');
});
});

describe('renderEnvironmentVariables', () => {
it('should set useNativeIntl to false if the environment variable is not true', () => {
expect(renderEnvironmentVariables('')).toMatchInlineSnapshot(`
"<script id="environment-variables" >
window.useNativeIntl = false
</script>"
`);
});

it('should set useNativeIntl to true is the environment variable is true', () => {
process.env.ONE_CONFIG_USE_NATIVE_INTL = 'true';
expect(renderEnvironmentVariables('')).toMatchInlineSnapshot(`
"<script id="environment-variables" >
window.useNativeIntl = true
</script>"
`);
});
});
});
26 changes: 25 additions & 1 deletion docs/api/server/Environment-Variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ One App can be configured via Environment Variables:
* [`ONE_CLIENT_ROOT_MODULE_NAME`](#one_client_root_module_name) ⚠️
* [`ONE_CLIENT_CDN_URL`](#one_client_cdn_url) ⚠️
* [`ONE_CONFIG_ENV`](#one_config_env) ⚠️
* [`ONE_CONFIG_USE_NATIVE_INTL`](#one_config_use_native_intl)
* Running in Development
* [`NODE_ENV`](#node_env) ⚠️
* [`ONE_CLIENT_ROOT_MODULE_NAME`](#one_client_root_module_name) ⚠️
* [`ONE_CONFIG_ENV`](#one_config_env) ⚠️
* [`ONE_DANGEROUSLY_ACCEPT_BREAKING_EXTERNALS`](#ONE_DANGEROUSLY_ACCEPT_BREAKING_EXTERNALS)
* [`ONE_CSP_ALLOW_INLINE_SCRIPTS`](#ONE_CSP_ALLOW_INLINE_SCRIPTS)
* [`ONE_DANGEROUSLY_DISABLE_CSP`](#ONE_DANGEROUSLY_DISABLE_CSP)
* [`ONE_CONFIG_USE_NATIVE_INTL`](#one_config_use_native_intl)
* Server Settings
* [`HOLOCRON_SERVER_MAX_MODULES_RETRY`](#holocron_server_max_modules_retry)
* [`HOLOCRON_SERVER_MAX_SIM_MODULES_FETCH`](#holocron_server_max_sim_modules_fetch)
Expand Down Expand Up @@ -74,6 +76,7 @@ One App can be configured via Environment Variables:
* [`ONE_CLIENT_REPORTING_URL`](#one_client_reporting_url) ⚠️
* [`ONE_CLIENT_ROOT_MODULE_NAME`](#one_client_root_module_name) ⚠️
* [`ONE_CONFIG_ENV`](#one_config_env) ⚠️
* [`ONE_CONFIG_USE_NATIVE_INTL`](#one_config_use_native_intl)
* [`ONE_ENABLE_POST_TO_MODULE_ROUTES`](#one_enable_post_to_module_routes)
* [`ONE_MAP_POLLING_MAX`](#one_map_polling_max)
* [`ONE_MAP_POLLING_MIN`](#one_map_polling_min)
Expand Down Expand Up @@ -524,6 +527,27 @@ ONE_CONFIG_ENV=staging
ONE_CONFIG_ENV=undefined
```

## `ONE_CONFIG_USE_NATIVE_INTL`
**Runs In**
* ✅ Production
* ✅ Development

Feature flag to disable lean-intl polyfill.
This allows you to use modern intl features such as timezones, but will result in your application supporting fewer older browsers.

**Shape**
```bash
ONE_CONFIG_USE_NATIVE_INTL=Boolean
```
**Example**
```bash
ONE_CONFIG_USE_NATIVE_INTL=true
```
**Default Value**
```bash
ONE_CONFIG_USE_NATIVE_INTL=false
```

## `ONE_DANGEROUSLY_ACCEPT_BREAKING_EXTERNALS`

**Runs In**
Expand Down Expand Up @@ -824,4 +848,4 @@ OTEL_RESOURCE_ATTRIBUTES="foo=bar;baz=qux"
[`HTTPS_PRIVATE_KEY_PASS_FILE_PATH`]: #https_private_key_pass_file_path
[`HTTPS_PORT`]: #https_port
[OTel Environment Variable Specification]: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/
[OTel Resource SDK documentation]: https://opentelemetry.io/docs/specs/otel/resource/sdk/#specifying-resource-information-via-an-environment-variable
[OTel Resource SDK documentation]: https://opentelemetry.io/docs/specs/otel/resource/sdk/#specifying-resource-information-via-an-environment-variable
8 changes: 4 additions & 4 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
"@americanexpress/env-config-utils": "^2.0.4",
"@americanexpress/fetch-enhancers": "^1.1.5",
"@americanexpress/one-app-bundler": "^6.21.1",
"@americanexpress/one-app-ducks": "^4.4.2",
"@americanexpress/one-app-ducks": "^4.5.0",
"@americanexpress/one-app-router": "^1.2.1",
"@americanexpress/vitruvius": "^3.0.1",
"@fastify/compress": "^6.4.0",
Expand Down
6 changes: 4 additions & 2 deletions src/client/polyfill/Intl.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@

import Intl from 'lean-intl';

global.Intl = Intl;
global.IntlPolyfill = Intl;
if (!(window && window.useNativeIntl)) {
global.Intl = Intl;
global.IntlPolyfill = Intl;
}
2 changes: 1 addition & 1 deletion src/client/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function initializeClientStore() {

export function loadPrerenderScripts(initialState) {
const locale = initialState && initialState.getIn(['intl', 'activeLocale']);
return locale ? getLocalePack(locale) : Promise.resolve();
return locale && !window?.useNativeIntl ? getLocalePack(locale) : Promise.resolve();
}

export function moveHelmetScripts() {
Expand Down
10 changes: 10 additions & 0 deletions src/server/config/env/runTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,16 @@ const runTime = [
validate: (value) => { if (!value) { throw new Error('The `ONE_CLIENT_ROOT_MODULE_NAME` environment variable must be defined.'); } },
defaultValue: () => (process.env.NODE_ENV === 'development' ? argv.rootModuleName : undefined),
},
{
name: 'ONE_CONFIG_USE_NATIVE_INTL',
defaultValue: 'false',
normalize: (input) => {
if (input?.toLowerCase() === 'true') {
return 'true';
}
return 'false';
},
},
{
name: 'ONE_REFERRER_POLICY_OVERRIDE',
defaultValue: () => '',
Expand Down
14 changes: 13 additions & 1 deletion src/server/plugins/reactHtml/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ const legacyBrowserChunkAssets = getChunkAssets(readJsonFile('../../../.build-me
.map((chunkAsset) => `legacy/${chunkAsset}`);

function renderI18nScript(clientInitialState, appBundlesURLPrefix) {
if (process.env.ONE_CONFIG_USE_NATIVE_INTL === 'true') {
return '';
}

const i18nFile = getI18nFileFromState(clientInitialState);
if (!i18nFile) {
return '';
Expand Down Expand Up @@ -236,6 +240,12 @@ export function getHead({
`;
}

export function renderEnvironmentVariables(nonce) {
Copy link
Contributor

Choose a reason for hiding this comment

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

just a naming nitpick, renderEnvironmentVariables indicates that its a standard practice to include, right now this is okay but could lead to issues in the future

return `<script id="environment-variables" ${nonce}>
window.useNativeIntl = ${process.env.ONE_CONFIG_USE_NATIVE_INTL === 'true'}
</script>`;
}

export function getBody({
isLegacy,
helmetInfo,
Expand All @@ -253,13 +263,14 @@ export function getBody({
const bundle = isLegacy ? 'legacyBrowser' : 'browser';
const { bodyAttributes, script } = helmetInfo;
const bundlePrefixForBrowser = isLegacy ? `${appBundlesURLPrefix}/legacy` : appBundlesURLPrefix;
const nonce = scriptNonce ? `nonce="${scriptNonce}"` : '';
return `
<body${(bodyAttributes && ` ${bodyAttributes.toString()}`) || ''}>
<div id="root">${appHtml || ''}</div>
${disableScripts
? ''
: `
<script id="initial-state" ${scriptNonce ? `nonce="${scriptNonce}"` : ''}>
<script id="initial-state" ${nonce}>
window.__webpack_public_path__ = ${jsonStringifyForScript(`${appBundlesURLPrefix}/`)};
window.__CLIENT_HOLOCRON_MODULE_MAP__ = ${jsonStringifyForScript(clientModuleMapCache[bundle])};
window.__INITIAL_STATE__ = ${jsonStringifyForScript(serializeClientInitialState(clientInitialState, request))};
Expand All @@ -268,6 +279,7 @@ export function getBody({
window.__render_mode__ = '${renderMode}';
window.__HOLOCRON_EXTERNALS__ = ${jsonStringifyForScript(getRequiredExternalsRegistry())};
</script>
${renderEnvironmentVariables(nonce)}
${assets}
${renderI18nScript(clientInitialState, bundlePrefixForBrowser)}
Matthew-Mallimo marked this conversation as resolved.
Show resolved Hide resolved
${renderExternalFallbacks({
Expand Down
6 changes: 5 additions & 1 deletion src/server/polyfill/intl.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@
* permissions and limitations under the License.
*/

global.Intl = require('lean-intl');
import Intl from 'lean-intl';

if (process.env.ONE_CONFIG_USE_NATIVE_INTL !== 'true') {
global.Intl = Intl;
}
Loading