Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.56.0] Cannot import re-exported named symbols #20083

Closed
3 tasks done
haggholm opened this issue Jul 7, 2018 · 10 comments
Closed
3 tasks done

[0.56.0] Cannot import re-exported named symbols #20083

haggholm opened this issue Jul 7, 2018 · 10 comments
Labels
Platform: Linux Building on Linux. Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot. Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used.

Comments

@haggholm
Copy link

haggholm commented Jul 7, 2018

Environment

  React Native Environment Info:
    System:
      OS: Linux 4.15 Ubuntu 18.04 LTS (Bionic Beaver)
      CPU: x64 AMD FX(tm)-6300 Six-Core Processor
      Memory: 3.38 GB / 31.32 GB
      Shell: 4.4.19 - /bin/bash
    Binaries:
      Node: 10.4.0 - ~/.nvm/versions/node/v10.4.0/bin/node
      npm: 6.1.0 - ~/.nvm/versions/node/v10.4.0/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      Android SDK:
        Build Tools: 23.0.1, 25.0.0, 25.0.2, 25.0.3, 26.0.1, 26.0.2, 27.0.3
        API Levels: 23, 25, 26, 27
    npmPackages:
      @client/react: ^1.0.13 => 1.0.63 
      @client/react-native: ^1.0.13 => 1.0.63 
      react-native: ^0.56.0 => 0.56.0 

Description

0.56 is exciting times! After getting past #19955 and then thanks to the changes in other modules prompted by @Ashoat in #20038, I’m finally able to build a JS bundle. However, it immediately crashes. The exact crash is irrelevant and uninteresting, but what it comes down to is this:

import { Component } from '@client/react';

// This fails because Component is undefined
class Foo extends Component {}

@client/react is a do-nothing shim that we use as a mechanism for synchronizing the version; it simply depends on a specific version of React (currently 16.4.1) looks like this:

import _default from 'react';
export default _default;
export * from 'react';

Possibly ugly, but I thought fairly harmless. However, just as in my previous issues (#19955, #20038), this works fine with Webpack, and worked fine with RN 0.55, but fails in RN 0.56.

If the Component class is explicitly re-exported, it works, and the default export works; but the generic re-export syntax export * from does not. As we use this to re-export quite a lot of symbols, it would not be trivial to update all those references (and it would change our shim from a fairly innocent do-nothing to something requiring active maintenance when symbol names change).

@haggholm
Copy link
Author

haggholm commented Jul 7, 2018

Does the bot just automatically add the “Old Version” tag to every new issue?

@haggholm
Copy link
Author

Looking at the generated index.android.bundle, I identified a file that re-exports symbols:

/* @flow */

import _default from 'react';
export default _default;
export * from 'react';

This was transpiled into:

__d(function (global, _$$_REQUIRE, module, exports, _dependencyMap) {
  Object.defineProperty(exports, "__esModule", {
    value: true
  });
  var _exportNames = {};
  exports.default = void 0;

  var _react = _interopRequireDefault(_$$_REQUIRE(_dependencyMap[0], "react"));

  Object.keys(_react).forEach(function (key) {
    if (key === "default" || key === "__esModule") return;
    if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
    Object.defineProperty(exports, key, {
      enumerable: true,
      get: function get() {
        return _react[key];
      }
    });
  });

  function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

  var _default2 = _react.default;
  exports.default = _default2;
},328,[594],"index.js");

"react" is not an ES module, so _interopRequireDefault() will return { default: react }, where React’s module.exports points to React. But then the only key on _react is default, hence no named symbols get exported.

Am I missing or misreading anything?

If not, the next question is why building with RN generates this…

@smerat
Copy link

smerat commented Jul 11, 2018

Same issue here

@reganperkins
Copy link

I don't have any details to add but am also hitting this issue and very stuck.

@haggholm
Copy link
Author

It looks like—I’m groping in unknown territory here—for some reason it’s using _interopRequireDefault() but should use _interopRequireWildcard() when re-exporting…? I guess this must be going wrong somewhere in @babel/plugin-transform-modules-commonjs.

@haggholm
Copy link
Author

Minimal repro: https://github.com/haggholm/babel-re-export-failure

@haggholm
Copy link
Author

See Babel issue.

I now realise that the reason we have not had this problem with our web build, but only React Native, is not an issue with RN per se, but due to the fact that Webpack can consume ES modules, so we do not perform a CommonJS module transform and hence don’t have any problematic Babel helpers in the Webpack build.

@kelset kelset added the Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used. label Jul 13, 2018
@haggholm
Copy link
Author

This has now been fixed in Babel (master); presumably it will be released in their next beta version (54). I hope that’s soon enough to go in the next RN point release…

@haggholm
Copy link
Author

The bug is fixed in the latest babel beta.54. Hopefully this will end up in the next RN release!

@hramos
Copy link
Contributor

hramos commented Aug 8, 2018

Metro 0.42.2 has this fix, and we're now on Metro 0.43+. Closing.

@hramos hramos closed this as completed Aug 8, 2018
@hramos hramos added Resolution: Fixed A PR that fixes this issue has been merged. and removed ⏪Old Version labels Aug 8, 2018
@facebook facebook locked as resolved and limited conversation to collaborators Aug 8, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: Linux Building on Linux. Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot. Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used.
Projects
None yet
Development

No branches or pull requests

6 participants