Improves the logic to import objects in helpers by ifsnow · Pull Request #10161 · babel/babel

ifsnow · 2019-07-04T09:25:47Z

Q	A
Fixed Issues?	No
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	No
Tests Added + Pass?	Yes
Documentation PR Link	N/A
Any Dependency Changes?	No
License	MIT

In the case of react-native,

import React, { PureComponent } from 'react';

Compiled as below.

var _react = _interopRequireWildcard(_$$_REQUIRE(_dependencyMap[7], "react"));

The below logic(looping) is performed whenever _interopRequireWildcard with react is called.

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];
      }
    }
  }
}

Generally, We have a lot of components that imports react or react-native. I don't think it's good to check the same object repeatedly on every call. so, I thought it would be better to put cache logic in here.

I counted the number of how many loops are executed to start my production-level App. It may feel like, but it feels like my app is a little faster. :)

Version	Count
Current	11,032
New (Cache)	128

I don't fully understand Babel architecture. Please let me know if I have any mistakes.

I hope this helps.

babel-bot · 2019-07-04T09:33:15Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11288/

babel-bot · 2019-07-04T09:33:15Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11044/

nicolo-ribaudo

This makes it more compliant to the node implementation:

// main.mjs

import * as a from "./a.js";
import { aFromB } from "./b.mjs";

console.log(a === aFromB);

// m.mjs

import * as a from "./a.js";

export { a as aFromB };

// a.js

module.exports = { foo: 1 };

packages/babel-helpers/src/helpers.js

nicolo-ribaudo · 2019-07-04T09:51:52Z

Note that this optimization would only work when using @babel/plugin-transform-runtime, otherwise the map is re-created in every file.

ifsnow · 2019-07-04T10:06:49Z

@nicolo-ribaudo Thanks for your feedback. I will try something better based on what you said.

ifsnow · 2019-07-05T05:38:23Z

@nicolo-ribaudo I've modified it to work well in the case you mentioned. Please let me know if there's anything else I need to consider.

packages/babel-helpers/src/helpers.js

ifsnow · 2019-07-16T04:41:10Z

@nicolo-ribaudo @Andarist @zloirock Thank you all for the feedback.

I realized that there might be no global object. https://github.com/webpack/webpack/blob/master/buildin/global.js#L16

In this case, I modified the cache to not work. I'm sure cache works well in modern development environments.

packages/babel-helpers/src/helpers.js

ifsnow · 2019-07-16T04:58:37Z

@ljharb Thank you for pointing out what I did not think of.

packages/babel-helpers/src/helpers.js

ifsnow · 2019-08-08T02:44:05Z

@nicolo-ribaudo @jridgewell @Andarist @zloirock @ljharb @cpojer Do you have plans to merge this PR?

I'm sure this cache feature makes React Native apps run faster. The following table shows the time spent running js bundle at startup.

Device + Engine	Current (ms)	Cache (ms)	Improving
Android 6.0 (32bit) + JSC	3,182	2,990	6%🔻
Android 7.0 (64bit) + JSC	1,118	1,089	2.6%🔻
Android 6.0 (32bit) + Hermes	1,230	964	21.7%🔻
Android 7.0 (64bit) + Hermes	403	367	9%🔻

The following cases are more effective.

Use many components
Use low performance devices
Use Hermes engine

I think we can improve the performance of many React Native apps without any effort. Please review it positively.

JLHwung · 2019-08-08T03:15:43Z

@ifsnow On the CircleCI failure, please rebase on upstream master.

Co-Authored-By: Jordan Harband <ljharb@gmail.com>

ifsnow · 2019-08-08T05:48:47Z

@JLHwung Thanks for your feedback. Rebased my changes on upstream master to clear CircleCI failure. I think Travis CI failure is due to a build system issue (Network timeout).

nicolo-ribaudo · 2019-08-08T06:20:09Z

Restarted

ifsnow · 2019-08-28T00:54:43Z

@nicolo-ribaudo Do you have any plans to release the new version? I'm wondering when this fix will be available :)

nicolo-ribaudo reviewed Jul 4, 2019

View reviewed changes

packages/babel-helpers/src/helpers.js Outdated Show resolved Hide resolved

nicolo-ribaudo added area: helpers PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories labels Jul 4, 2019

nicolo-ribaudo reviewed Jul 14, 2019

View reviewed changes

packages/babel-helpers/src/helpers.js Outdated Show resolved Hide resolved

ljharb reviewed Jul 16, 2019

View reviewed changes

packages/babel-helpers/src/helpers.js Outdated Show resolved Hide resolved

nicolo-ribaudo reviewed Jul 16, 2019

View reviewed changes

packages/babel-helpers/src/helpers.js Show resolved Hide resolved

nicolo-ribaudo approved these changes Jul 16, 2019

View reviewed changes

jridgewell approved these changes Jul 20, 2019

View reviewed changes

packages/babel-helpers/src/helpers.js Outdated Show resolved Hide resolved

ljharb approved these changes Aug 8, 2019

View reviewed changes

JLHwung approved these changes Aug 8, 2019

View reviewed changes

ifsnow and others added 7 commits August 8, 2019 13:54

Improves the logic to import objects in helpers

b7d2192

Modified to work with more cases.

2e46b8d

Modified by considering exception cases

8c4bb12

Fixed not using bundler's polyfill

7829928

Co-Authored-By: Jordan Harband <ljharb@gmail.com>

Updated test output files

24ef760

Changes to cache logic

86cafc9

Modified to use if statement

5c622ec

ifsnow force-pushed the fix/improve-import-wild branch from 2e0b029 to 5c622ec Compare August 8, 2019 05:11

nicolo-ribaudo merged commit 9ec26a7 into babel:master Aug 8, 2019

samMeow mentioned this pull request Oct 16, 2019

babel transfrom runtime incompatible with url loader #10561

Closed

theKashey mentioned this pull request Oct 19, 2019

_interopRequireWildcard breaks on nodejs cases #10577

Closed

ExE-Boss mentioned this pull request Oct 24, 2019

feat(importStar): Cache non‑module results microsoft/tslib#80

Open

snyk-bot mentioned this pull request Oct 24, 2019

[Snyk] Upgrade @babel/runtime from 7.4.4 to 7.6.3 Matthelonianxl/drizzle#9

Open

snyk-bot mentioned this pull request Nov 1, 2019

[Snyk] Upgrade @babel/core from 7.2.2 to 7.6.4 nbv1982/material-ui#5

Open

Hypnosphi mentioned this pull request Nov 9, 2019

Angular Storyshots 5.0.6: Cannot read property 'get' of undefined storybookjs/storybook#6490

Closed

snyk-bot mentioned this pull request Nov 12, 2019

[Snyk] Upgrade @babel/runtime from 7.1.5 to 7.6.3 ajesse11x/Parse-SDK-JS#1

Open

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Nov 27, 2019

lock bot locked as resolved and limited conversation to collaborators Nov 27, 2019

Uh oh!

Conversation

ifsnow commented Jul 4, 2019

Uh oh!

babel-bot commented Jul 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Jul 4, 2019

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicolo-ribaudo commented Jul 4, 2019

Uh oh!

ifsnow commented Jul 4, 2019

Uh oh!

ifsnow commented Jul 5, 2019

Uh oh!

Uh oh!

ifsnow commented Jul 16, 2019

Uh oh!

Uh oh!

ifsnow commented Jul 16, 2019

Uh oh!

Uh oh!

Uh oh!

ifsnow commented Aug 8, 2019

Uh oh!

JLHwung commented Aug 8, 2019

Uh oh!

ifsnow commented Aug 8, 2019

Uh oh!

nicolo-ribaudo commented Aug 8, 2019

Uh oh!

ifsnow commented Aug 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

babel-bot commented Jul 4, 2019 •

edited

Loading