Closed Bug 1712447 Opened 5 months ago Closed 2 months ago

Use typescript definitions for redux and friends in frontend

Categories

(Webtools Graveyard :: Pontoon, task)

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: axel, Assigned: axel)

References

Details

Attachments

(1 file)

44 bytes, text/x-github-pull-request
Details | Review

redux and redux-thunk both have type definitions built in, which the frontend code doesn't use.

Filing this as a separate bug, as I can see this become an avalanche throughout most of the frontend code-base.

I made an attempt at getting the global app state typed and hit a point where I needed a deeper knowledge of both Redux and Typescript. Here is a summary of where I got to.

Redux has documentation on using Typescript, however, it is built on the assumption that the project also uses Redux Toolkit. Pontoon does not use this so I had to do a bit more digging/interpreting.

After installing the @types for redux and redux-react I attempted to type the global store state. When making some adjustments in RootState to allow Typescript to infer the state from combineReducers() it can detect the shape but not specifics so the state ends up looking like

Reducer<CombinedState<{
    [x: string]: ...;
    router: RouterState<unknown>;
}>

This appears to work OK for components that still use mapStateToProps() however it does not work when using the useSelector() hook. In those instances a TS error is thrown and the app is not able to be built:

TypeScript error in /app/frontend/src/core/editor/components/EditorMainAction.tsx(28,26):
#27 101.3 Property 'editor' does not exist on type 'DefaultRootState'.  TS2339
#27 101.3 
#27 101.3     26 | ): React.ReactElement<React.ElementType> {
#27 101.3     27 |     const isRunningRequest = useSelector(
#27 101.3   > 28 |         (state) => state.editor.isRunningRequest,
#27 101.3        |                          ^
#27 101.3     29 |     );
#27 101.3     30 |     const forceSuggestions = useSelector(
#27 101.3     31 |         (state) => state.user.settings.forceSuggestions,
#27 101.3 
#27 101.3 
#27 101.5 error Command failed with exit code 1.

After some research I found this post which states that the inferred typing did not work for them and they had to do some custom typing. When I follow that suggestion I'm able to get the type to recognize the different state types as a Union type but the useSelector() hook is then not able to recognize anything deeper within that state.

I no longer have the redux/redux-react @types in my local environment for me to display the output here but for the above error it can find state.entity but it gives the same error at state.entity.isRunningRequest stating that it does not exist on AppState (the name I gave to the global store state).

Given that using the custom typing from the SO post helped in finding the initial shape of state I feel like it would be possible to adjust that in a way that would allow TS to understand the overall shape of our state. I just don't have the experience in TS and/or Redux to tackle this myself.

I guess that reducers in rootReducers.ts is part of the problem. It ends up mapping strings to a union type.

I dabbled into a thing:

diff --git a/frontend/src/core/editor/index.ts b/frontend/src/core/editor/index.ts
index 5b06cd55..d91e38c4 100644
--- a/frontend/src/core/editor/index.ts
+++ b/frontend/src/core/editor/index.ts
@@ -25,4 +25,4 @@ export type { Translation } from './actions';

 // Name of this module.
 // Used as the key to store this module's reducer.
-export const NAME: string = 'editor';
+export const NAME = 'editor';

That might help. On my way to bed, if nobody beats me to it, I might do that across the board to see if that helps.

After some useless errands like "should we update TypeScript if it cannot detect the const names", I looked at what NAME actually was, and recalled that I stumbled upon that before. Might have been with action constants.
typeof const NAME: string is any const string, typeof const NAME is just that one string literal.

:shrug:

Attached file GitHub Pull Request

PR landed (awfully, but still)

Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Assignee: nobody → axel
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.