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

Updated front-build to v12 #354

Merged
merged 2 commits into from
Aug 11, 2022
Merged

Updated front-build to v12 #354

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb4069b
chore: updated frontend-build version & resolved eslint issues
BilalQamar95 Aug 5, 2022
3730ac2
fix: useMemo hook removed from JSX, eslint issue ignored
BilalQamar95 Aug 10, 2022
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
4 changes: 2 additions & 2 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// eslint-disable-next-line import/no-extraneous-dependencies
const { getBaseConfig } = require('@edx/frontend-build');

const config = getBaseConfig('eslint');

config.rules = {
'import/no-extraneous-dependencies': ['error', {
'devDependencies': [
devDependencies: [
'**/*.config.js',
'**/*.test.jsx',
'**/*.test.js',
Expand All @@ -19,7 +20,6 @@ config.rules = {
}],
'jsx-a11y/anchor-is-valid': ['error', {
components: ['Link'],
specialLink: [],
specialLink: ['to'],
aspects: ['noHref', 'invalidHref', 'preferButton'],
}],
Expand Down
12,021 changes: 5,598 additions & 6,423 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"homepage": "https://github.com/edx/frontend-platform#readme",
"devDependencies": {
"@edx/brand": "npm:@edx/brand-openedx@1.1.0",
"@edx/frontend-build": "9.2.2",
"@edx/frontend-build": "^12.0.3",
"@edx/paragon": "20.2.0",
"axios-mock-adapter": "1.20.0",
"core-js": "3.21.1",
Expand Down
2 changes: 1 addition & 1 deletion src/auth/MockAuthService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ class MockAuthService {
}

return this.getAuthenticatedUser();
})
});

/**
* A Jest mock function (jest.fn())
Expand Down
2 changes: 1 addition & 1 deletion src/auth/interceptors/createRetryInterceptor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const createRetryInterceptor = (options = {}) => {
try {
const backoffDelay = getBackoffMilliseconds(nthRetry);
// Delay (wrapped in a promise so we can await the setTimeout)
await new Promise(resolve => setTimeout(resolve, backoffDelay));
await new Promise(resolve => { setTimeout(resolve, backoffDelay); });
// Make retry request
retryResponse = await httpClient.request(config);
} catch (e) {
Expand Down
26 changes: 13 additions & 13 deletions src/i18n/countries.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@ import { getPrimaryLanguageSubtag } from './lib';
* TODO: When we start dynamically loading translations only for the current locale, change this.
*/

COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ar.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/en.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/es.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/fr.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/zh.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ca.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/he.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/id.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ko.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/pl.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/pt.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ru.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ar'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/en'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/es'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/fr'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/zh'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ca'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/he'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/id'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ko'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/pl'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/pt'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/ru'));
// COUNTRIES.registerLocale(require('i18n-iso-countries/langs/th.json')); // Doesn't exist in lib.
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/uk.json'));
COUNTRIES.registerLocale(require('i18n-iso-countries/langs/uk'));

/**
* Provides a lookup table of country IDs to country names for the current locale.
Expand Down
10 changes: 5 additions & 5 deletions src/i18n/languages.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ import { getPrimaryLanguageSubtag } from './lib';
*/

// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/ar.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/en.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/es.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/fr.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/en'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/es'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/fr'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/zh.json'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/ca.json'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/he.json'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/id.json'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/ko.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/pl.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/pt.json'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/pl'));
LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/pt'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/ru.json'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/th.json'));
// LANGUAGES.registerLocale(require('@cospired/i18n-iso-languages/langs/uk.json'));
Expand Down
8 changes: 5 additions & 3 deletions src/react/AppProvider.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useState, useMemo } from 'react';
import PropTypes from 'prop-types';
import { Router } from 'react-router-dom';

Expand Down Expand Up @@ -65,11 +65,13 @@ export default function AppProvider({ store, children }) {
<IntlProvider locale={locale} messages={getMessages()}>
<ErrorBoundary>
<AppContext.Provider
value={{ authenticatedUser, config, locale }}
value={
useMemo(() => ({ authenticatedUser, config, locale }), [authenticatedUser, config, locale])
Copy link
Contributor

Choose a reason for hiding this comment

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

As a point of style, I'd prefer to remove this useMemo hook from the JSX and create an intermediate variable that we then pass into the value attribute here.

const appContextValue = useMemo(() => ({ authenticatedUser, config, locale }), [authenticatedUser, config, locale]);

... That sorta thing.

}
>
<OptionalReduxProvider store={store}>
<Router history={history}>
<>{children}</>
{children}
</Router>
</OptionalReduxProvider>
</AppContext.Provider>
Expand Down
1 change: 1 addition & 0 deletions src/react/AuthenticatedPageRoute.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable react/jsx-no-constructed-context-values */
import React from 'react';
import { mount } from 'enzyme';
import { Router, Route } from 'react-router-dom';
Expand Down
4 changes: 1 addition & 3 deletions src/react/OptionalReduxProvider.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import { Provider } from 'react-redux';
*/
export default function OptionalReduxProvider({ store, children }) {
if (store === null) {
return (
<>{children}</>
);
return children;
}

return (
Expand Down
2 changes: 1 addition & 1 deletion src/react/PageRoute.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default function PageRoute(props) {
if (match) {
sendPageEvent();
}
}, [JSON.stringify(match)]);
Copy link
Contributor

Choose a reason for hiding this comment

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

This change makes me nervous - are we sure that the match object changes with the same frequency as the stringified version of it? I'd be worried that we end up sending page events far more frequently, which could be pretty disruptive to our analytics. If this was just to satisfy the linter I'd probably revert this and leave it as-is. We should try not to make code changes when just updating the build library.

}, [match]);

return (
<Route {...props} />
Expand Down
2 changes: 1 addition & 1 deletion src/react/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ export const useAppEvent = (type, callback) => {
return function cleanup() {
unsubscribe(subscriptionToken);
};
}, []);
}, [callback, type]);
};