Update/react imports #670

nicholasio · 2021-01-20T00:09:43Z

What:

Updates React imports as per the new JSX transform (React 17). #668 .

Why:

React does not need to be in scope anymore with React 17 and the new JSX transform.
How:

npx react-codemod update-react-imports and selecting the Typescript option. ESLint errors were ignored. Also had to update the babel config for jest for the unit tests to pass.

Tasks:

Unrelated Tasks

Additional Comments

In some places we have imports like this:

import { MouseEvent, useEffect, useRef, useCallback } from "react";
import * as React from "react";

This is basically because in some places we are using React.FC instead of importing FC directly. I noticed we're not very consistent on how we import TypeScript types, it seems like the React codemod prefers React.FC as it didn't update them and instead converted the react imports to the new recommended version import * as React from 'react'

changeset-bot · 2021-01-20T00:09:46Z

🦋 Changeset detected

Latest commit: bcb5c04

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages

Name	Type
@frontity/analytics	Minor
@frontity/components	Minor
@frontity/comscore-analytics	Minor
@frontity/connect	Minor
@frontity/core	Minor
frontity	Minor
@frontity/google-ad-manager	Minor
@frontity/google-analytics	Minor
@frontity/google-tag-manager-analytics	Minor
@frontity/head-tags	Minor
@frontity/hooks	Minor
@frontity/html2react	Minor
@frontity/mars-theme	Minor
@frontity/smart-adserver	Minor
@frontity/twentytwenty-theme	Minor
@frontity/types	Minor
@frontity/yoast	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2021-01-20T00:15:04Z

Codecov Report

Merging #670 (bcb5c04) into dev (0d67328) will increase coverage by 0.01%.
The diff coverage is 81.82%.

Impacted Files	Coverage Δ
packages/components/iframe.tsx	`100.00% <ø> (ø)`
packages/components/image.tsx	`100.00% <ø> (ø)`
packages/components/script.tsx	`0.00% <0.00%> (ø)`
packages/core/src/app/index.tsx	`0.00% <ø> (ø)`
packages/core/src/client/index.tsx	`0.00% <ø> (ø)`
packages/core/src/server/index.tsx	`0.00% <ø> (ø)`
packages/frontity/src/utils/slot-and-fill/slot.tsx	`100.00% <ø> (ø)`
...ges/frontity/src/utils/slot-and-fill/use-fills.tsx	`100.00% <ø> (ø)`
...ackages/google-ad-manager/src/components/index.tsx	`0.00% <ø> (ø)`
packages/google-analytics/src/components/index.tsx	`100.00% <ø> (ø)`
... and 12 more

luisherranz · 2021-01-20T10:17:59Z

npx react-codemod update-react-imports and selecting the Typescript option.

That's a good one 😄

It seems like some of the e2e tests are failing. We need to investigate that. Were you able to take a look @nicholasio?

nicholasio · 2021-01-20T11:27:30Z

npx react-codemod update-react-imports and selecting the Typescript option.

That's a good one 😄

It seems like some of the e2e tests are failing. We need to investigate that. Were you able to take a look @nicholasio?

Noticed that, will investigate that, had some issues yesterday with e2e tests not running but I'll figure it out.

nicholasio · 2021-01-20T13:03:12Z

It looks like the update to React 17 and the new JSX transform is not quite working, frontity is still requiring react to be in scope.

nicholasio · 2021-01-21T12:23:12Z

FYI - I'm still looking into this.

cristianbote · 2021-01-21T13:46:53Z

It looks like the update to React 17 and the new JSX transform is not quite working, frontity is still requiring react to be in scope.

@nicholasio really sorry for this one. Do you know what fails? Or where exactly is the import needed? 🤔

nicholasio · 2021-01-21T15:05:31Z

It looks like the update to React 17 and the new JSX transform is not quite working, frontity is still requiring react to be in scope.

@nicholasio really sorry for this one. Do you know what fails? Or where exactly is the import needed? 🤔

Apparently, the new JSX transform is not being used, I know that babel will only use if it's available (even when you set to automatic so perhaps when frontity compiles the app, babel can´t find the jsx transform.

cristianbote · 2021-01-22T07:53:03Z

@nicholasio I've look a bit more into it and applying the codemod on this branch #666 will make everything work smoothly.

I believe the problem lies in the React 17 and emotion 10 not working smoothly by default. So, would you mind applying the mods on that branch instead?

nicholasio · 2021-01-22T12:29:40Z

@cristianbote Nice! I'll wait for that branch to get merged then 👍

…date/react-imports

nicholasio · 2021-01-23T01:58:16Z

@cristianbote I just merged your branch here things seem to be working now, I got a couple of e2e tests failing locally but I think they are unrelated.

cristianbote · 2021-01-25T07:09:20Z

@nicholasio I've just merged in the emotion 11 PR and now things should be smoother. Thank you so much for your contribution! ✌️

cristianbote · 2021-01-25T11:26:20Z

.changeset/honest-apples-guess.md

+"@frontity/analytics": minor
+"@frontity/components": minor
+"@frontity/comscore-analytics": minor
+"@frontity/connect": minor
+"@frontity/core": minor
+"frontity": minor
+"@frontity/google-ad-manager": minor
+"@frontity/google-analytics": minor
+"@frontity/google-tag-manager-analytics": minor
+"@frontity/head-tags": minor
+"@frontity/hooks": minor
+"@frontity/html2react": minor
+"@frontity/mars-theme": minor
+"@frontity/smart-adserver": minor
+"@frontity/twentytwenty-theme": minor
+"@frontity/types": minor
+"@frontity/yoast": minor


@nicholasio was thinking if these updates require a minor update or rather a patch instead? These are not changing public facing API but rather the environment the code is running on. What do you think?

That's a good point, I think I didn't go with a patch because this is not exactly a bug fix, but I'm fine to change it to patch

cristianbote · 2021-01-26T07:55:59Z

@nicholasio thank you so much for working on this, but can you try and solve the merge conflicts above? 🙏 afterwards I believe this is gonna be ready to be merged in 🎉 .

If there's anything I can do to help, please let me know. Thank you!

nicholasio · 2021-01-26T11:43:27Z

@nicholasio thank you so much for working on this, but can you try and solve the merge conflicts above? 🙏 afterwards I believe this is gonna be ready to be merged in 🎉 .

If there's anything I can do to help, please let me know. Thank you!

Merge conflicts have been fixed!

cristianbote · 2021-01-26T11:44:40Z

@nicholasio thank you so much for working on this, but can you try and solve the merge conflicts above? 🙏 afterwards I believe this is gonna be ready to be merged in 🎉 .
If there's anything I can do to help, please let me know. Thank you!

Merge conflicts have been fixed!

Awesome! Waiting for tests and then we'll approve and merge. Thank you! 😃

cristianbote · 2021-01-26T13:51:09Z

Boom! 🎉 Thank you @nicholasio!

nicholasio added 3 commits January 19, 2021 20:51

Updating React Imports

3709480

Updating jest babel config to support the new JSX transform

a4edf02

Adding Changeset

3ced7fd

nicholasio self-assigned this Jan 21, 2021

Merge remote-tracking branch 'upstream/chore/adopt-emotion11' into up…

3bfec06

…date/react-imports

Merge branch 'dev' into update/react-imports

2d51975

cristianbote self-requested a review January 25, 2021 08:03

cristianbote and others added 3 commits January 25, 2021 12:49

Merge branch 'dev' into update/react-imports

088b919

Merge remote-tracking branch 'upstream/dev' into dev

4fad6a4

Merge branch 'dev' into update/react-imports

80d4883

cristianbote added the contribution label Jan 25, 2021

cristianbote reviewed Jan 25, 2021

View reviewed changes

nicholasio added 2 commits January 26, 2021 08:19

Merge remote-tracking branch 'upstream/dev' into dev

7c2199e

Merge branch 'dev' into update/react-imports

bcb5c04

cristianbote approved these changes Jan 26, 2021

View reviewed changes

cristianbote merged commit be1cae9 into frontity:dev Jan 26, 2021

cristianbote linked an issue Jan 26, 2021 that may be closed by this pull request

Remove old React imports from the themes #668

Closed

frontibotito mentioned this pull request Jan 27, 2021

Version Packages #681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update/react imports #670

Update/react imports #670

nicholasio commented Jan 20, 2021 •

edited

changeset-bot bot commented Jan 20, 2021 •

edited

codecov bot commented Jan 20, 2021 •

edited

luisherranz commented Jan 20, 2021 •

edited

nicholasio commented Jan 20, 2021

nicholasio commented Jan 20, 2021

nicholasio commented Jan 21, 2021

cristianbote commented Jan 21, 2021

nicholasio commented Jan 21, 2021 •

edited

cristianbote commented Jan 22, 2021

nicholasio commented Jan 22, 2021

nicholasio commented Jan 23, 2021

cristianbote commented Jan 25, 2021

cristianbote Jan 25, 2021

nicholasio Jan 25, 2021

cristianbote commented Jan 26, 2021

nicholasio commented Jan 26, 2021

cristianbote commented Jan 26, 2021

cristianbote commented Jan 26, 2021

Update/react imports #670

Update/react imports #670

Conversation

nicholasio commented Jan 20, 2021 • edited

changeset-bot bot commented Jan 20, 2021 • edited

🦋 Changeset detected

codecov bot commented Jan 20, 2021 • edited

Codecov Report

luisherranz commented Jan 20, 2021 • edited

nicholasio commented Jan 20, 2021

nicholasio commented Jan 20, 2021

nicholasio commented Jan 21, 2021

cristianbote commented Jan 21, 2021

nicholasio commented Jan 21, 2021 • edited

cristianbote commented Jan 22, 2021

nicholasio commented Jan 22, 2021

nicholasio commented Jan 23, 2021

cristianbote commented Jan 25, 2021

cristianbote Jan 25, 2021

Choose a reason for hiding this comment

nicholasio Jan 25, 2021

Choose a reason for hiding this comment

cristianbote commented Jan 26, 2021

nicholasio commented Jan 26, 2021

cristianbote commented Jan 26, 2021

cristianbote commented Jan 26, 2021

nicholasio commented Jan 20, 2021 •

edited

changeset-bot bot commented Jan 20, 2021 •

edited

codecov bot commented Jan 20, 2021 •

edited

luisherranz commented Jan 20, 2021 •

edited

nicholasio commented Jan 21, 2021 •

edited