Upgrade to Babel 7 #1191

brunolemos · 2018-11-28T18:11:56Z

Some tests are failing:

Test Suites: 5 failed, 33 passed, 38 total
Tests:       32 failed, 316 passed, 348 total
Snapshots:   15 failed, 3 obsolete, 73 passed, 88 total

Might be related: jestjs/jest#7016, jestjs/jest#7118

Closes #1170

necolas

Thanks for looking into this. Asked questions inline.

Every AsyncStorage test is failing. There's probably a common cause. It might be that the way assertions are done needs to change following the Jest upgrade.

It looks like these changes have broken the Babel transform as the output is being compiled when it shouldn't be.

FAIL  packages/babel-plugin-react-native-web/src/__tests__/index-test.js
  ● Rewrite react-native to react-native-web › import from "native-native"
    expect(value).toMatchSnapshot()
    Received value does not match stored snapshot "Rewrite react-native to react-native-web import from "native-native": import from "native-native" 1".
    - Snapshot
    + Received
    @@ -4,12 +4,17 @@
      import { Invalid, View as MyView, ViewPropTypes } from 'react-native';
      import * as ReactNativeModules from 'react-native';
      
            ↓ ↓ ↓ ↓ ↓ ↓
      
    - import ReactNative from 'react-native-web/dist/index';
    - import View from 'react-native-web/dist/exports/View';
    - import { Invalid } from 'react-native-web/dist/index';
    - import MyView from 'react-native-web/dist/exports/View';
    - import ViewPropTypes from 'react-native-web/dist/exports/ViewPropTypes';
    - import * as ReactNativeModules from 'react-native-web/dist/index';
    + "use strict";
    + 
    + var ReactNativeModules = _interopRequireWildcard(require("react-native-web/dist/index"));
    + 
    + var _View = _interopRequireDefault(require("react-native-web/dist/exports/View"));
    + 
    + var _ViewPropTypes = _interopRequireDefault(require("react-native-web/dist/exports/ViewPropTypes"));
    + 
    + function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
    + 
    + function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) { var desc = Object.defineProperty && Object.getOwnPropertyDescriptor ? Object.getOwnPropertyDescriptor(obj, key) : {}; if (desc.get || desc.set) { Object.defineProperty(newObj, key, desc); } else { newObj[key] = obj[key]; } } } } newObj.default = obj; return newObj; } }
      "
      at tester (node_modules/babel-plugin-tester/dist/index.js:196:44)
      at Object.<anonymous> (node_modules/babel-plugin-tester/dist/index.js:108:13)
      at step (node_modules/babel-plugin-tester/dist/index.js:37:191)
      at node_modules/babel-plugin-tester/dist/index.js:37:361

Some of the failures look like Jest changed the signature of some functions.

FAIL  packages/react-native-web/src/exports/TextInput/__tests__/index-test.js (12.328s)
  ● components/TextInput › prop "clearTextOnFocus" › encountered a declaration exception
    Missing second argument. It must be a callback function.
      13 |     test(message, fn);
      14 |   } else {
    > 15 |     test.skip(`${message} – document is not focused`);
         |          ^
      16 |   }
      17 | };

FAIL  packages/react-native-web/src/exports/Text/__tests__/index-test.js
  ● components/Text › encountered a declaration exception
    Missing second argument. It must be a callback function.
      35 |   });
      36 | 
    > 37 |   test('prop "numberOfLines"');

And this failure makes me think the way the native methods are applied to the class prototype isn't working anymore

● components/TextInput › prop "onKeyPress" › enter key
    TypeError: _this.blur is not a function
      402 |       }
      403 |       if (shouldBlurOnSubmit) {
    > 404 |         this.blur();
          |              ^
      405 |       }
      406 |     }
      407 |   };

packages/babel-plugin-react-native-web/package.json

necolas · 2018-11-28T18:34:32Z

package.json

@@ -49,8 +51,8 @@
    "flow-bin": "^0.63.1",
    "glob": "^7.1.2",
    "husky": "^0.14.3",
-    "jest": "^22.4.3",
-    "jest-canvas-mock": "^1.0.2",
+    "jest": "^23.6.0",


Jest needs to be upgraded to upgrade Babel?

I just expected better babel 7 support with the latest version. I noticed jest was still installing the babel 6 dependencies but it kept doing it after the upgrade.

Reverted, but had to change a config var testURL to http://localhost/ because it was giving an error without this (ps: it's the default on newer jests).

necolas · 2018-11-28T18:34:43Z

package.json

+    "@babel/preset-env": "^7.1.6",
+    "@babel/preset-flow": "^7.0.0",
+    "@babel/preset-react": "^7.0.0",
+    "babel-core": "^7.0.0-bridge.0",


What is this for?

Not sure if it's required, it was automatically added by https://github.com/babel/babel-upgrade

It is so that babel-jest can work. https://jestjs.io/docs/en/getting-started#using-babel

necolas · 2018-11-28T18:34:54Z

package.json

+    "@babel/preset-react": "^7.0.0",
+    "babel-core": "^7.0.0-bridge.0",
+    "babel-eslint": "^10.0.1",
+    "babel-jest": "^23.6.0",


Why is this needed now?

This was also added automatically by https://github.com/babel/babel-upgrade

Without this I get invalid syntax errors when it sees a import.

It seems this causes jest to compile the code, but based on your comment that shouldn't happen.

Might that be because you removed the babelrc file? Jest might have been using that before

The .babelrc was migrated to babel.config.js, this change was required to fix something.

.flowconfig

scripts/jest/config.js

brunolemos · 2018-11-28T19:25:34Z

@necolas is BABEL_ENV=commonjs required? Removing it makes it stop converting the import to require.

necolas · 2018-12-03T15:15:10Z

Are you planning to fix the test failures?

brunolemos · 2018-12-03T18:13:16Z

packages/react-native-web/src/exports/TextInput/index.js

@@ -401,7 +401,7 @@ class TextInput extends Component<*> {
        onSubmitEditing(e);
      }
      if (shouldBlurOnSubmit) {
-        this.blur();
+        if (this.blur) this.blur();


The tests were failing with _this.blur is not a function, not sure what's the best way to fix is, so I changed this code.

brunolemos · 2018-12-03T18:15:28Z

@necolas build is passing now.

Please make the remaining changes, if any. I may not be available.

necolas · 2018-12-03T22:36:58Z

Thank you! Do you know which update fixed the AsyncStorage tests?

necolas

Notes for myself are included inline

necolas · 2018-12-03T22:38:03Z

packages/babel-plugin-react-native-web/src/__tests__/__snapshots__/index-test.js.snap

+\\"use strict\\";
+
+exports.__esModule = true;
+exports.createElement = exports.Text = exports.StyleSheet = exports.ColorPropType = exports.View = void 0;


Ensure these snapshots are not changed. The commonjs flag is probably no longer needed here: https://github.com/necolas/react-native-web/blob/0.9.8/package.json#L18

This was being caused by the babel-core bridge dependency

necolas · 2018-12-03T22:38:39Z

packages/react-native-web/src/exports/TextInput/index.js

@@ -401,7 +401,7 @@ class TextInput extends Component<*> {
        onSubmitEditing(e);
      }
      if (shouldBlurOnSubmit) {
-        this.blur();
+        if (this.blur) this.blur();


Get to the bottom of what causes this to fail

brunolemos · 2018-12-03T22:55:34Z

@necolas: Thank you! Do you know which update fixed the AsyncStorage tests?

Yes, it was caused by a new version of jsdom, 11.12.0 I think. It "freezes" the window object, so when you did window.localStorage = mockXXX in the test it wasn't having any effect.

SimenB · 2018-12-11T22:46:28Z

package.json

+    "@babel/preset-react": "^7.0.0",
+    "babel-core": "^7.0.0-bridge.0",
+    "babel-eslint": "^10.0.0",
+    "babel-jest": "^23.4.2",


make sure to match jest major version here

Fix #1170 Close #1205 Close #1191

necolas · 2018-12-20T17:11:37Z

This is done in branch react-native-0.57

Fix #1170 Close #1205 Close #1191

brunolemos · 2018-12-20T23:36:02Z

Nice, would appreciate git cherry-pick being used next time

necolas · 2018-12-21T00:33:06Z

Sorry in this case I didn't use this PR as the basis for the work as I had to change all the dependencies and fix the tests rather than commit incorrect an snapshot and leave the blur instance method broken

Fix #1170 Close #1205 Close #1191

necolas reviewed Nov 28, 2018

View reviewed changes

brunolemos force-pushed the babel branch 2 times, most recently from fc4c0dc to 3d734d5 Compare November 28, 2018 19:10

colincclark mentioned this pull request Dec 3, 2018

Build problems with babel 7 for react-native(-web) dependencies using flow vercel/next.js#5288

Closed

brunolemos force-pushed the babel branch 3 times, most recently from ea02f35 to 7ce7d3c Compare December 3, 2018 18:04

Upgrade to Babel 7

0bf736d

brunolemos force-pushed the babel branch from 7ce7d3c to 0bf736d Compare December 3, 2018 18:10

brunolemos commented Dec 3, 2018

View reviewed changes

necolas requested changes Dec 3, 2018

View reviewed changes

colincclark mentioned this pull request Dec 4, 2018

Upgrade to babel 7 newsuk/times-components#1540

Closed

SimenB reviewed Dec 11, 2018

View reviewed changes

Repository owner deleted a comment from hyochan Dec 15, 2018

necolas added a commit that referenced this pull request Dec 20, 2018

[change] Compile using Babel 7

d152d30

Fix #1170 Close #1205 Close #1191

necolas closed this Dec 20, 2018

necolas added a commit that referenced this pull request Dec 20, 2018

[change] Compile using Babel 7

7e53737

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Dec 21, 2018

[change] Compile using Babel 7

766572f

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Dec 30, 2018

[change] Compile using Babel 7

38598f9

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Dec 30, 2018

[change] Compile using Babel 7

c31a030

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Dec 31, 2018

[change] Compile using Babel 7

89c2a5d

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Dec 31, 2018

[change] Compile using Babel 7

0d54eea

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Jan 1, 2019

[change] Compile using Babel 7

4f56d70

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Jan 1, 2019

[change] Compile using Babel 7

b65743f

Fix #1170 Close #1205 Close #1191

necolas added a commit that referenced this pull request Jan 23, 2019

[change] Compile using Babel 7

77d4bd8

Fix #1170 Close #1205 Close #1191

brunolemos deleted the babel branch July 11, 2019 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Babel 7 #1191

Upgrade to Babel 7 #1191

brunolemos commented Nov 28, 2018

necolas left a comment

necolas Nov 28, 2018

brunolemos Nov 28, 2018 •

edited

Loading

necolas Nov 28, 2018

brunolemos Nov 28, 2018

colincclark Dec 3, 2018 •

edited

Loading

necolas Nov 28, 2018

brunolemos Nov 28, 2018 •

edited

Loading

necolas Nov 28, 2018

brunolemos Nov 28, 2018

brunolemos commented Nov 28, 2018 •

edited

Loading

necolas commented Dec 3, 2018

brunolemos Dec 3, 2018

brunolemos commented Dec 3, 2018 •

edited

Loading

necolas commented Dec 3, 2018

necolas left a comment

necolas Dec 3, 2018

necolas Dec 20, 2018

necolas Dec 3, 2018

brunolemos commented Dec 3, 2018 •

edited

Loading

SimenB Dec 11, 2018

necolas commented Dec 20, 2018

brunolemos commented Dec 20, 2018

necolas commented Dec 21, 2018

Upgrade to Babel 7 #1191

Upgrade to Babel 7 #1191

Conversation

brunolemos commented Nov 28, 2018

necolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunolemos Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colincclark Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunolemos Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunolemos commented Nov 28, 2018 • edited Loading

necolas commented Dec 3, 2018

Choose a reason for hiding this comment

brunolemos commented Dec 3, 2018 • edited Loading

necolas commented Dec 3, 2018

necolas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunolemos commented Dec 3, 2018 • edited Loading

Choose a reason for hiding this comment

necolas commented Dec 20, 2018

brunolemos commented Dec 20, 2018

necolas commented Dec 21, 2018

brunolemos Nov 28, 2018 •

edited

Loading

colincclark Dec 3, 2018 •

edited

Loading

brunolemos Nov 28, 2018 •

edited

Loading

brunolemos commented Nov 28, 2018 •

edited

Loading

brunolemos commented Dec 3, 2018 •

edited

Loading

brunolemos commented Dec 3, 2018 •

edited

Loading