Updated front-build to v12 #354
Conversation
Codecov Report
@@ Coverage Diff @@
## master #354 +/- ##
==========================================
+ Coverage 81.39% 81.41% +0.02%
==========================================
Files 38 38
Lines 919 920 +1
Branches 170 170
==========================================
+ Hits 748 749 +1
Misses 159 159
Partials 12 12
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
src/react/AppProvider.jsx
Outdated
| @@ -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]) | |||
There was a problem hiding this comment.
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.
| @@ -22,7 +22,7 @@ export default function PageRoute(props) { | |||
| if (match) { | |||
| sendPageEvent(); | |||
| } | |||
| }, [JSON.stringify(match)]); | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think this commit should be a fix: since we're touching code that runs in our apps - it's not just a chore update of the build library.
See comments inline, there's one thing I think I'd factor a bit differently, and another which I'd hesitate to include in this PR.
Ticket:
42: Upgrade eslint to v8.x
Description
frontend-buildto v12 (Eslint was updated infrontend-buildversion resulting in it's version being bumped to v12. This PR updatesfrontend-buildto reciprocate eslint version update)eslintissues post updateMerge checklist:
frontend-platform. This can be done by runningnpm startand opening http://localhost:8080.module.config.jsfile infrontend-build.fix,feat) and is appropriate for your code change. Consider whether your code is a breaking change, and modify your commit accordingly.Post merge: