Catch attempts to import * as x from react-native

[robhogan]

Motivation

Motivated by some very misleading redbox errors. When my IDE (Webstorm) slipped up auto-generating an import line, I ended up with this in my code:
import * as Platform from 'react-native'

The result:

Android (with remote debugging enabled)

Navigator is deprecated and has been removed 
from this package. It can now be installed and 
imported from `react-native-deprecated-custom-
components` instead of `react-native`. Learn about 
alternative navigation solutions at http://facebook
.github.io/react-native/docs/navigation.html`

iOS

Native module cannot be null.

This is because import * ... triggers the getters in react-native-implementation.js, some of which throw errors for various reasons. These error messages had me chasing ghosts until I noticed my bad import line.

(NB: Android without remote debugging didn't throw any errors at all - I guess the getters are called lazily?)

Test Plan

The situation above now results in the following redbox on iOS and android:

Release Notes

[GENERAL] [ENHANCEMENT] [Libraries/react-native/react-native-implementation.js] - Better error reporting when attempting to import * from react-native

[timabilov]

.

[facebook-github-bot]

@rh389 I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[facebook-github-bot]

@rh389 I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[robhogan]

I don’t know who best to review a developer experience issue - maybe @hramos ?

This is a minor thing really, just one of those I thought I might as well deal with after it stung me, but the thumbs on react-navigation/react-navigation#1328 (comment) suggest I’m not the only one who’s been caught out.

[facebook-github-bot]

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[hramos]

Can you rebase?

[empyrical]

What if these deprecated/erroring modules had the property descriptor enumerable: false, so you could still import * without calling those getters?

Here's a quick example against Babel's _interopRequireWildcard that works without throwing an error:

const fakeModule = { 'foo': 'bar' };

Object.defineProperty(fakeModule, 'invisible', {
  enumerable: false,
  get() {
    throw new Error('Bad getter called');
  }
});

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)) newObj[key] = obj[key]; } } newObj.default = obj; return newObj; } }

const fakeImport = _interopRequireWildcard(fakeModule);

[robhogan]

Seems like a much better solution to me

[empyrical]

I took a stab at having deprecated exports (not just Navigator) be unenumerable, but I discovered a side-effect of how import is transpiled that makes that a bit more complicated

Say you wanted to use the wildcard import:

import * as ReactNative from 'react-native';

ReactNative.SomeApi.someThing();

Then in a later release SomeApi is deprecated. Since it's unenumerable, what I thought would be nice is have it so it could still be manually imported while you transition away:

import * as ReactNative from 'react-native';
import {SomeApi} from 'react-native';

SomeApi.someThing();

Babel 6 in this fiddler transpiles this to:

var _reactNative = require('react-native');

var ReactNative = _interopRequireWildcard(_reactNative);

// snipped helper definition

_reactNative.SomeApi.someThing();

But Babel 7 in Metro transpiles it to:

  var ReactNative = babelHelpers.interopRequireWildcard(_$$_REQUIRE(_dependencyMap[0], "react-native"));
  ReactNative.SomeApi.someThing();

The big important difference here is in the first example, it's trying to get SomeApi right off of react native's export object, but in the second example if you try to wildcard import from the module then it will try to get SomeApi off of the wildcard export object. Since it was declared unenumerable, it won't be available.

I'm not sure if this is a babel 7 issue or a metro babel plugin issue. Will try and look into this later.

[empyrical]

Actually - it appears as if this stuff isn't needed after all. _interopRequireWildcard was updated in Babel 7 so it will copy getter functions over without calling them! So import * as RN from 'react-native' will work without error in RN 0.57

[cpojer]

Based on @empyrical's comment, I'll close this PR, as it seems like with babel7 this won't be necessary any longer. While I don't personally recommend using import *, that's a subjective thing and everyone should be able to use it as much as they'd like to :)