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.

Sign up for GitHub

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

Jump to bottom

feat: Initial changes to support OEP65 #535

Closed
wants to merge 3 commits into from
Closed

feat: Initial changes to support OEP65 #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a0e625a
feat: Initial changes to support OEP65
pedromartello Aug 1, 2023
c21ece7
fix: Adressing Lint errors from CI Checks
pedromartello Aug 25, 2023
f6b6347
fix(tests): augmented tests to cover new features
pedromartello Aug 25, 2023
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
927 changes: 765 additions & 162 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"@edx/brand": "npm:@edx/brand-openedx@1.2.0",
"@edx/browserslist-config": "1.2.0",
"@edx/frontend-build": "12.9.10",
"@edx/paragon": "^20.44.0",
"@edx/paragon": "^21.1.2",
"@testing-library/react-hooks": "^8.0.1",
"@wojtekmaj/enzyme-adapter-react-17": "0.8.0",
"axios-mock-adapter": "^1.21.3",
Expand All @@ -47,7 +47,7 @@
"prop-types": "15.8.1",
"react": "17.0.2",
"react-dom": "17.0.2",
"react-redux": "7.2.9",
"react-redux": "^8.1.1",
"react-router-dom": "^6.6.1",
"redux": "4.2.1",
"regenerator-runtime": "0.14.0"
Expand All @@ -70,7 +70,7 @@
"lodash.merge": "4.6.2",
"lodash.snakecase": "4.1.1",
"pubsub-js": "1.9.4",
"react-intl": "^5.25.0",
"react-intl": "^6.4",
"universal-cookie": "4.0.4"
},
"peerDependencies": {
Expand All @@ -79,7 +79,7 @@
"prop-types": "^15.7.2",
"react": "^16.9.0 || ^17.0.0",
"react-dom": "^16.9.0 || ^17.0.0",
"react-redux": "^7.1.1",
"react-redux": "^8.1.1",
"react-router-dom": "^6.0.0",
"redux": "^4.0.4"
}
Expand Down
1 change: 1 addition & 0 deletions src/i18n/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export {
getMessages,
isRtl,
handleRtl,
mergeMessages,
LOCALE_CHANGED,
LOCALE_TOPIC,
} from './lib';
Expand Down
11 changes: 7 additions & 4 deletions src/i18n/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,15 @@ const optionsShape = {
/**
*
*
* @param {Array} [messagesArray=[]]
* @param {Object} newMessages
* @returns {Object}
* @memberof module:Internationalization
*/
export function mergeMessages(messagesArray = []) {
return Array.isArray(messagesArray) ? merge({}, ...messagesArray) : {};
export function mergeMessages(newMessages) {
const msgs = Array.isArray(newMessages) ? merge({}, ...newMessages) : newMessages;
messages = merge(messages, msgs);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing this would change the expected behavior of the function. Do we know if any other packages/MFEs use it?

Maybe it's best to create a new function for this purpose, just in case. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see my comment below


return messages;
}

/**
Expand All @@ -262,7 +265,7 @@ export function configure(options) {
loggingService = options.loggingService;
// eslint-disable-next-line prefer-destructuring
config = options.config;
messages = Array.isArray(options.messages) ? mergeMessages(options.messages) : options.messages;
messages = Array.isArray(options.messages) ? merge({}, ...options.messages) : options.messages;
Copy link
Contributor Author

@pedromartello pedromartello Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@arbrandes - the only use of the mergeMessages function was here (and in the unit tests). Note how it has now been changed to use lodash.merge directly. The exports from @edx/frontend-platform and @edx/frontend-platform/i18n were added in this commit.


if (config.ENVIRONMENT !== 'production') {
Object.keys(messages).forEach((key) => {
Expand Down
68 changes: 68 additions & 0 deletions src/i18n/lib.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,58 @@ describe('lib', () => {
});

describe('mergeMessages', () => {
it('should merge objects', () => {
configure({
loggingService: { logError: jest.fn() },
config: {
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
messages: {
ar: { message: 'ar-hah' },
},
});
const result = mergeMessages({ en: { foo: 'bar' }, de: { buh: 'baz' }, jp: { gah: 'wut' }});
expect(result).toEqual({
ar: { message: 'ar-hah' },
en: { foo: 'bar' },
de: { buh: 'baz' },
jp: { gah: 'wut' },
});
});

it('should merge objects from an array', () => {
configure({
loggingService: { logError: jest.fn() },
config: {
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
messages: {
ar: { message: 'ar-hah' },
},
});
const result = mergeMessages([{ foo: 'bar' }, { buh: 'baz' }, { gah: 'wut' }]);
expect(result).toEqual({
ar: { message: 'ar-hah' },
foo: 'bar',
buh: 'baz',
gah: 'wut',
});
});

it('should merge nested objects from an array', () => {
configure({
loggingService: { logError: jest.fn() },
config: {
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
messages: {
en: { init: 'initial' },
es: { init: 'inicial' },
},
});
const messages = [
{
en: { hello: 'hello' },
Expand All @@ -274,19 +316,45 @@ describe('mergeMessages', () => {
const result = mergeMessages(messages);
expect(result).toEqual({
en: {
init: 'initial',
hello: 'hello',
goodbye: 'goodbye',
},
es: {
init: 'inicial',
hello: 'hola',
goodbye: 'adiós',
},
});
});

it('should return an empty object if no messages', () => {
configure({
loggingService: { logError: jest.fn() },
config: {
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
messages: {},
});
expect(mergeMessages(undefined)).toEqual({});
expect(mergeMessages(null)).toEqual({});
expect(mergeMessages([])).toEqual({});
expect(mergeMessages({})).toEqual({});
});

it('should return the original object if no messages', () => {
configure({
loggingService: { logError: jest.fn() },
config: {
ENVIRONMENT: 'production',
LANGUAGE_PREFERENCE_COOKIE_NAME: 'yum',
},
messages: { en: { hello: 'world ' } },
});
expect(mergeMessages(undefined)).toEqual({ en: { hello: 'world ' } });
expect(mergeMessages(null)).toEqual({ en: { hello: 'world ' } });
expect(mergeMessages([])).toEqual({ en: { hello: 'world ' } });
expect(mergeMessages({})).toEqual({ en: { hello: 'world ' } });
});
});
4 changes: 4 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ export {
initializeMockApp,
mockMessages,
} from './testing';
export {
defineMessages,
mergeMessages,
} from './i18n';
18 changes: 9 additions & 9 deletions src/initialize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,15 @@ export async function initialize({
await handlers.logging();
publish(APP_LOGGING_INITIALIZED);

// Internationalization
configureI18n({
messages,
config: getConfig(),
loggingService: getLoggingService(),
});
await handlers.i18n();
publish(APP_I18N_INITIALIZED);

// Authentication
configureAuth(authServiceImpl, {
loggingService: getLoggingService(),
Expand All @@ -335,15 +344,6 @@ export async function initialize({
await handlers.analytics();
publish(APP_ANALYTICS_INITIALIZED);

// Internationalization
configureI18n({
messages,
config: getConfig(),
loggingService: getLoggingService(),
});
await handlers.i18n();
publish(APP_I18N_INITIALIZED);

// Application Ready
await handlers.ready();
publish(APP_READY);
Expand Down