chore: upgrade React; use new JSX transform; sort imports #1129

zwliew · 2021-04-13T07:04:53Z

Problem

Let's upgrade to the new JSX transform to stay modern.

React has introduced the new JSX transform to enable improvements in future React releases. Although it does nothing significant as of now, it might be a good idea to upgrade anyway to stay up-to-date.

Reference on transitioning to the new JSX transform: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html

Solution

Improvements:

Upgrade React to v16.14.0, which adds support for the new JSX transform
Use the new JSX transform
Uninstall TypeScript from the root directory as it causes VSCode/CoC's tsserver to use it for static analysis instead of the one in the frontend directory
Convert all default React import statements to destructured imports (import React from 'react' -> import { ... } from 'react')

Before & After Screenshots

No visual changes.

Tests

These tests have been verified locally but not on Amplify staging yet.

Deploy the change on Amplify.
Conduct smoke tests:

Create and send SMS campaign
Create and send Telegram campaign
Create and send email campaign
Create and send protected email campaign

All pages and components should work fine without errors.

Deploy Notes

Upgraded dependencies:

react, react-dom: 16.31.1 -> 16.14.0
@types/react: 16.9.29 -> 16.14.5
@types/react-dom: 16.9.5 -> 16.9.12

package.json

frontend/tsconfig.json

miazima

Lgtm, just need to shift the modules import order

miazima · 2021-04-27T04:19:08Z

frontend/src/components/common/action-button/ActionButton.tsx

 import cx from 'classnames'

 import styles from './ActionButton.module.scss'

+import type { MouseEvent as ReactMouseEvent } from 'react'


Let's move this up top together with the react import, combine them if possible?

miazima · 2021-04-27T04:27:58Z

frontend/src/components/common/body-wrapper/BodyWrapper.tsx

 import cx from 'classnames'

 import styles from './BodyWrapper.module.scss'

+import type { FunctionComponent } from 'react'
+


To follow the import order convention, external module imports should be at the top. We could also look into if we want to use this plugin to sort import order
https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/order.md#importorder-enforce-a-convention-in-module-import-order

Thanks! I agree about sorting the import order with a plugin. I've looked into both eslint-plugin-import and eslint-plugin-simple-import-sort and settled on the eslint-plugin-import since it better supports module aliasing.

zwliew · 2021-05-03T01:52:55Z

I've just rebased the branch atop develop.

Just to clarify, eslint-plugin-import sorts imports into the following groups, with newlines between each group:

builtin modules (e.g. Node.js modules)
external modules (e.g. react)
parent relative models (e.g. ../xyz)
sibling modules (e.g. ./abc)
index file (e.g. ./index.ts)
import-mapped modules (e.g. classes, components)

miazima · 2021-05-04T04:43:08Z

@lamkeewei I had suggested using an eslint plugin to auto sort import order for our frontend, wanted to get your go ahead too

lamkeewei · 2021-05-04T04:46:00Z

@lamkeewei I had suggested using an eslint plugin to auto sort import order for our frontend, wanted to get your go ahead too

Sure! Will make our imports neater. Might also be useful to think about what is a good sort order for our imports, beyond the default rules (e.g. assets should be first/last, etc). Can refer to what vaccine ops did for their sorting.

We don't need this in the root directory.

This release introduces support for the new JSX transform. Changelog: https://github.com/facebook/react/blob/master/CHANGELOG.md#16140-october-14-2020

Reference: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#eslint

Reference: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#removing-unused-react-imports

Also, use `import type` where it makes sense. Note: React's ``MouseEvent` type conflicts with the global `MouseEvent` type, so rename it to `ReactMouseEvent` as well.

The group order is: 1. builtin modules (e.g. Node) 2. external modules (e.g. react) 3. parent relative models (e.g. ../xyz) 4. sibling modules (e.g. ./abc) 5. index file (e.g. ./index.ts) 6. import-mapped modules (e.g. classes, components)

zwliew · 2021-05-07T06:04:38Z

Rebased the branch.

Todo:
look into making assets a group and moving them to the end of the imports list. This doesn't seem possible with eslint-plugin-import/order, so we will likely need to use eslint-plugin-simple-import-sort for this. vaccine ops uses it as well.

However, I don't feel like this is a big deal. The existing import order seems good to me.

miazima

The sorted import order is sufficient for now, improvising it has been delegated to a separate issue

* develop: fix: fix error when updating Telegram ID for an existing phone number (#1178) chore: upgrade React; use new JSX transform; sort imports (#1129) fix(backend): docker build and tsc output directory structure (#1177) feat: refactor msg template components; add telegram character limit (#1148) refactor: use shared function to initialize models (#1172) chore: setup scaffolding for backend tests (#940) 1.23.0 fix(frontend): fix frontend test flakiness (#1162) fix: update successful delivery status only if error does not exist (#1150) chore: upgrade dependencies (#1153)

* develop: chore: upgrade dependencies (#1180) Backend tests - updating template for channels (#1175) feat(rich-text): support image as links (#1158) fix: fix error when updating Telegram ID for an existing phone number (#1178) chore: upgrade React; use new JSX transform; sort imports (#1129)

zwliew commented Apr 13, 2021

View reviewed changes

package.json Show resolved Hide resolved

zwliew commented Apr 13, 2021

View reviewed changes

frontend/tsconfig.json Show resolved Hide resolved

miazima reviewed Apr 27, 2021

View reviewed changes

zwliew changed the title ~~chore: upgrade React and use the new JSX transform~~ chore: upgrade React; use new JSX transform; use import-sort plugin Apr 28, 2021

zwliew changed the title ~~chore: upgrade React; use new JSX transform; use import-sort plugin~~ chore: upgrade React; use new JSX transform; sort imports Apr 28, 2021

zwliew force-pushed the feat-jsx-transform branch from 6398174 to 3d5d8f7 Compare May 3, 2021 01:49

zwliew added 9 commits May 7, 2021 11:59

chore(root): uninstall typescript from root directory

e0f2523

We don't need this in the root directory.

chore(react): upgrade to v16.14.0

94c9e0b

This release introduces support for the new JSX transform. Changelog: https://github.com/facebook/react/blob/master/CHANGELOG.md#16140-october-14-2020

chore(typescript): enable the new JSX transform

3b94ca2

chore(react): upgrade type definitions

cfb432c

fix(eslint): disable rules regarding JSX imports

6d60113

Reference: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#eslint

chore(frontend): apply React codemod to remove React imports

f787cc5

Reference: https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html#removing-unused-react-imports

chore(react): replace global imports with destructured imports

06eedc5

Also, use `import type` where it makes sense. Note: React's ``MouseEvent` type conflicts with the global `MouseEvent` type, so rename it to `ReactMouseEvent` as well.

chore: install and enable eslint-plugin-import

ab39f5b

zwliew force-pushed the feat-jsx-transform branch from 3d5d8f7 to 6508ce8 Compare May 7, 2021 05:56

zwliew mentioned this pull request May 7, 2021

Sort imports using eslint-plugin-import/eslint-plugin-simple-import-sort #1179

Closed

miazima self-requested a review May 10, 2021 02:51

miazima approved these changes May 10, 2021

View reviewed changes

miazima merged commit cedad15 into develop May 10, 2021

miazima deleted the feat-jsx-transform branch May 10, 2021 02:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade React; use new JSX transform; sort imports #1129

chore: upgrade React; use new JSX transform; sort imports #1129

zwliew commented Apr 13, 2021 •

edited

Loading

miazima left a comment

miazima Apr 27, 2021

miazima Apr 27, 2021

zwliew Apr 28, 2021 •

edited

Loading

zwliew commented May 3, 2021

miazima commented May 4, 2021

lamkeewei commented May 4, 2021

zwliew commented May 7, 2021 •

edited

Loading

miazima left a comment

chore: upgrade React; use new JSX transform; sort imports #1129

chore: upgrade React; use new JSX transform; sort imports #1129

Conversation

zwliew commented Apr 13, 2021 • edited Loading

Problem

Solution

Before & After Screenshots

Tests

Deploy Notes

miazima left a comment

Choose a reason for hiding this comment

miazima Apr 27, 2021

Choose a reason for hiding this comment

miazima Apr 27, 2021

Choose a reason for hiding this comment

zwliew Apr 28, 2021 • edited Loading

Choose a reason for hiding this comment

zwliew commented May 3, 2021

miazima commented May 4, 2021

lamkeewei commented May 4, 2021

zwliew commented May 7, 2021 • edited Loading

miazima left a comment

Choose a reason for hiding this comment

zwliew commented Apr 13, 2021 •

edited

Loading

zwliew Apr 28, 2021 •

edited

Loading

zwliew commented May 7, 2021 •

edited

Loading