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.49.1] Broken bundling on run-ios in simulator #16208

Closed
fungilation opened this issue Oct 5, 2017 · 20 comments
Closed

[0.49.1] Broken bundling on run-ios in simulator #16208

fungilation opened this issue Oct 5, 2017 · 20 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@fungilation
Copy link

fungilation commented Oct 5, 2017

Is this a bug report?

Yes.

Upgraded RN to 0.49.1.

Terminal output after react-native run-ios:

React packager ready.

Loading dependency graph, done.
Launching Dev Tools...
Bundling `index.js`  [development, non-minified]  0.0% (0/1), failed.
error: bundling failed: Error
    at DependencyGraph._getAbsolutePath ([PROJECT]/node_modules/metro-bundler/src/node-haste/DependencyGraph.js:305:11)
    at DependencyGraph.getDependencies ([PROJECT]/node_modules/metro-bundler/src/node-haste/DependencyGraph.js:283:4236)
    at Resolver.getDependencies ([PROJECT]/node_modules/metro-bundler/src/Resolver/index.js:129:5)
    at [PROJECT]/node_modules/metro-bundler/src/Bundler/index.js:642:39
    at Generator.next (<anonymous>)
    at step ([PROJECT]/node_modules/metro-bundler/src/Bundler/index.js:13:1336)
    at [PROJECT]/node_modules/metro-bundler/src/Bundler/index.js:13:1496
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

And a corresponding redbox on simulator, for "Cannot find entry file index.js in any of the roots". Reloading simulator would just repeat the above.

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 8.6.0
Yarn: 1.1.0
npm: 5.3.0
Watchman: 4.9.0
Xcode: Xcode 9.0 Build version 9A235
Android Studio: Not Found

Packages: (wanted => installed)
react: 16.0.0-beta.5 => 16.0.0-beta.5
react-native: 0.49.1 => 0.49.1

Target Platform: iOS (11.0)

Steps to Reproduce

  1. react-native-git-upgrade
  2. react-native run-ios
  3. Bundling failure, as shown above

Expected Behavior

App bundles and run.

Actual Behavior

screenshot 2017-10-04 17 08 46

Also in case it's 3rd package issues (unlikely since it's not building at all), here's my package.json:

{
  "name": "[NAME]",
  "version": "0.0.1",
  "private": true,
  "eslintConfig": {
    "parserOptions": {
      "ecmaVersion": 6,
      "sourceType": "module",
      "ecmaFeatures": {
        "jsx": true,
        "experimentalObjectRestSpread": true
      }
    },
    "env": {
      "browser": true,
      "node": true
    },
    "plugins": [
      "react",
      "react-native"
    ],
    "rules": {
      "comma-dangle": [
        2,
        "always-multiline"
      ],
      "semi": [
        2,
        "never"
      ],
      "react-native/no-unused-styles": 2,
      "react-native/split-platform-components": 2
    }
  },
  "scripts": {
    "start": "node node_modules/react-native/local-cli/cli.js start"
  },
  "dependencies": {
    "babel-plugin-idx": "^2",
    "he": "^1.1.0",
    "jssha": "^2.3.1",
    "lodash": "^4.17.2",
    "moment-timezone": "^0.5.10",
    "node-summary": "file:../node-summary",
    "react": "16.0.0-beta.5",
    "react-native": "0.49.1",
    "react-native-blur": "^3.1",
    "react-native-code-push": "5.1.3-beta",
    "react-native-firebase": "^3.0",
    "react-native-fit-image": "^1.4.8",
    "react-native-highlight-words": "^1.0.0",
    "react-native-keep-awake": "^2.0.4",
    "react-native-linear-gradient": "^2.0.0",
    "react-native-modalbox": "^1.3.8",
    "react-native-notification": "^2.0.0",
    "react-native-orientation": "^3.0.0",
    "react-native-parallax-scroll-view": "file:../react-native-parallax-scroll-view",
    "react-native-safari-view": "^2.0.0",
    "react-native-sentry": "^0.26",
    "react-native-status-bar-size": "^0.3.2",
    "react-native-swiper": "^1.5.10",
    "react-native-tooltip": "^5.0.0",
    "react-native-tts": "^1.3.0",
    "react-native-vector-icons": "^4.1.1",
    "react-native-webview-bridge": "lefnire/react-native-webview-bridge",
    "react-redux": "^5.0.1",
    "redux": "^3.6.0",
    "redux-thunk": "^2.1.0"
  },
  "devDependencies": {
    "redux-logger": "^3.0.6"
  }
}

Reproducible Demo

Umm, am I the only one hitting this after all the RC's? I see some errors reported like this that's caused by 3rd party packages, but my terminal output says nothing about errring on a certain package, it just ends on line at process._tickCallback (internal/process/next_tick.js:188:7).

I would give more reproducing details if I even know where to look.

@iainkirkpatrick
Copy link

I'm seeing this too, with a similar upgrading RN to 0.49 situation. Fix for me was to duplicate my index.ios.js file and rename as index.js - the only indication of this I can see in the docs after a quick look is here.

Does this mean we are required to have a single index.js entry point from 0.49?

@fungilation
Copy link
Author

fungilation commented Oct 5, 2017

Interesting, will have to test that.

How is this not a breaking change unlisted in changelog if intentional? I don't think it is intentional as we still want iOS and android code separation.

@ghost
Copy link

ghost commented Oct 5, 2017

Experiencing as well.

fungilation referenced this issue Oct 5, 2017
Summary:
This change (initially discussed in expo/create-react-native-app#26) moves the HelloWorld project template from two nearly identical entry points (`index.android.js` and `index.ios.js`) to a single, minimal `index.js` entry point. The root component is created in `App.js`. This unifies the project structure between `react-native init` and Create React Native App and allows CRNA's eject to use the entry point from the HelloWorld template without any hacks to customize it. Also examples in the docs can be just copy-pasted to `App.js` the same way in both HelloWorld and CRNA apps without having to first learn about  `AppRegistry.registerComponent`.

* Created a new project from the template using `./scripts/test-manual-e2e.sh` and verified that:
  * The app builds, starts and runs both on Android and iOS.
  * Editing and reloading changes works.
  * The new files (`index.js`, `App.js`, `__tests__/App.js`) get created in the project folder.

<img width="559" alt="screen shot 2017-08-01 at 19 10 51" src="https://user-images.githubusercontent.com/497214/28835171-300a12b6-76ed-11e7-81b2-623639c3b8f6.png">
<img width="467" alt="screen shot 2017-08-01 at 19 09 12" src="https://user-images.githubusercontent.com/497214/28835180-33d285e0-76ed-11e7-8d68-2b3bc44bf585.png">

<!--
Thank you for sending the PR!

If you changed any code, please provide us with clear instructions on how you verified your changes work. In other words, a test plan is *required*. Bonus points for screenshots and videos!

Please read the Contribution Guidelines at https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md to learn more about contributing to React Native.

Happy contributing!
-->
Closes #15312

Differential Revision: D5556276

Pulled By: hramos

fbshipit-source-id: 068fdf7e51381c2bc50321522f2be0db47296c5e
@alexxpoch
Copy link

Same here

@Andreyco
Copy link
Contributor

Andreyco commented Oct 5, 2017

Interesting, will have to test that.

How is this not a breaking change unlisted in changelog if intentional? I don't think it is intentional as we still want iOS and android code separation.

While it's breaking change (should be listed), there is nothing preventing you from separating platform specific code.
I find it convenient to have single application entry point.

@fungilation
Copy link
Author

fungilation commented Oct 5, 2017

Can we get some core dev input on whether moving to index.js as new required entry point is intentional, so index.ios.js and index.android.js are now "retired"?

@grabbou @ide ?

@grabbou
Copy link
Contributor

grabbou commented Oct 5, 2017

It's not a breaking change since it should affect just new apps. See here: 6e99e31 - the commit modifies the HelloWorld app which is used as a template when using init.

I believe git-upgrade has an unwanted behaviour as it takes that change and applies it into your project whereas it doesn't touch filesystem: https://github.com/facebook/react-native/blob/master/local-cli/upgrade/upgrade.js#L139-L147

Either:

  1. Remove index.android.js and rename index.ios.js to index.js if two files are identical
  2. Revert that change and stick to two separate entry points

Please pay attention to git diff after upgrading. All changes should be at a glance. It's fine to revert them in AppDelegate.m and MainApplication.

@fungilation
Copy link
Author

  1. Revert that change and stick to two separate entry points

You mean revert to to RN 0.48? Ya that's what I did. The larger question is what's new convention going forward. I'm for just moving to index.js so I for one would just do that. But for RN, this needs:

  1. Documented as a breaking change, or
  2. Fix upgrade.js so it doesn't affect existing installation and allow separate ios and android indexes

Which should it be?

@Andreyco
Copy link
Contributor

Andreyco commented Oct 5, 2017

In my opinion, it definitely is breaking change. All existing apps does not work after upgrade. New ones are flawless, working as expected.

Thumbd up for single entry point! 👍🏻

@grabbou
Copy link
Contributor

grabbou commented Oct 5, 2017

Can you share with me your AppDelegate.m after upgrade?

It's an unwanted side-effect of an upgrade tool as far as I am aware. Once I get a breaking AppDelegate.m file, I will be able to make sure the issue is what I think. Next, I'll attach a special note in the release notes that folks should be aware of when upgrading.

@fungilation
Copy link
Author

fungilation commented Oct 5, 2017

Inside my AppDelegate.m
0.48.4:

#ifdef DEBUG
    jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index.ios" fallbackResource:nil];
#else
    jsCodeLocation = [CodePush bundleURL];
#endif

0.49.1:

#ifdef DEBUG
    jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
#else
    jsCodeLocation = [CodePush bundleURL];
#endif

Yes I manually merged since I have a conflicting change with CodePush that failed 3 way merge during react-native-git-upgrade. I also already tried index.ios on 0.49.1, same bundling failure.

@fungilation fungilation changed the title [0.48.1] Broken bundling on run-ios in simulator [0.49.1] Broken bundling on run-ios in simulator Oct 5, 2017
@grabbou
Copy link
Contributor

grabbou commented Oct 5, 2017 via email

@fungilation
Copy link
Author

fungilation commented Oct 5, 2017

Upgrading to 0.49.1 again. Note these changes merged by react-native-git-upgrade, are these issues too for forcing index.js? (on android side)

"Applied patch to 'android/app/build.gradle' cleanly."

Added:

project.ext.react = [
    entryFile: "index.js"
]

"Applied patch to 'android/app/src/main/java/com/[APP]/MainApplication.java' cleanly."

Added:

    @Override
    protected String getJSMainModuleName() {
      return "index";
    }

inside private final ReactNativeHost mReactNativeHost = new ReactNativeHost(this) {}

@Andreyco
Copy link
Contributor

Andreyco commented Oct 5, 2017

Patch for my upgrade, excluded some files to keep it cleaner.

As expected, upgrade process did change code to pick index.js as entry point.
I am not sure it would be possible to merge index.ios.js and index.android.js automatically, but probably.

I think it is sufficient to inform users about this change ;)

diff --git a/android/app/build.gradle b/android/app/build.gradle
index 8fab59d..77b7192 100644
--- a/android/app/build.gradle
+++ b/android/app/build.gradle
@@ -80,6 +80,10 @@ project.ext.envConfigFiles = [
 
 apply from: project(':react-native-config').projectDir.getPath() + "/dotenv.gradle"
 
+project.ext.react = [
+    entryFile: "index.js"
+]
+
 apply from: "../../node_modules/react-native/react.gradle"
 
 /**
diff --git a/android/app/src/main/java/com/app/MainApplication.java b/android/app/src/main/java/com/app/MainApplication.java
index 01d7c8c..ab01a1d 100644
--- a/android/app/src/main/java/com/app/MainApplication.java
+++ b/android/app/src/main/java/com/app/MainApplication.java
@@ -33,6 +33,11 @@ public class MainApplication extends Application implements ReactApplication {
         new ImagePickerPackage()
       );
     }
+
+    @Override
+    protected String getJSMainModuleName() {
+      return "index";
+    }
   };
 
   @Override
diff --git a/ios/app/AppDelegate.m b/ios/app/AppDelegate.m
index 4e1baef..a79f344 100644
--- a/ios/app/AppDelegate.m
+++ b/ios/app/AppDelegate.m
@@ -18,7 +18,7 @@
 {
   NSURL *jsCodeLocation;
 
-  jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index.ios" fallbackResource:nil];
+  jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
 
   RCTRootView *rootView = [[RCTRootView alloc] initWithBundleURL:jsCodeLocation
                                                       moduleName:@"app"

@fungilation
Copy link
Author

fungilation commented Oct 5, 2017

I confirm that bundler works on keeping index.ios in AppDelegate.m.

However my app is still broken hitting this error. I do have moment-timezone dependency.

Commenting out code that uses moment and its import, and I'm hitting another error (redbox) in https://github.com/alinz/react-native-webview-bridge, about PropTypes.func and PropTypes being null. I know many old libraries are still stuck on not having migrated away from React.PropTypes, like clauderic/react-native-highlight-words#2. But did RN 0.49 went and made React.PropTypes use a silent error with redbox now, instead of the warning Warning: PropTypes has been moved to a separate package... on previous RN releases? Or this is due to React being upgraded? Doesn't look like any such move in React from googling.

Yes I'm frustrated how many things break on a single version upgrade. Tip on whether React.PropTypes should remain a non-breaking warning in RN 0.49 would be appreciated.

@grabbou
Copy link
Contributor

grabbou commented Oct 8, 2017

I have added note about this issue to the release notes as well as tweeted about it. Should prevent it from further escalation.

Or this is due to React being upgraded? Doesn't look like any such move in React from googling.

This has been a change in React itself and so, is most likely a result of React being upgraded. Maybe we should be more explicit about its version bumps, especially when we change majors.

@zrven
Copy link

zrven commented Oct 10, 2017

react-native-git-upgrade updated with index.js. I changed AppDelegate.m to
jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index.ios" fallbackResource:nil]; which worked fine. [Look for @index.ios], earlier it was just @Index

@Reiji87
Copy link

Reiji87 commented Oct 11, 2017

screen shot 2017-10-11 at 3 52 16 pm

I keep getting the following error after upgrading from 0.47.0 to 0.49.0, is there a way to fix this?

@grabbou
Copy link
Contributor

grabbou commented Oct 11, 2017 via email

@grabbou
Copy link
Contributor

grabbou commented Oct 13, 2017

I am going to close this issue as the breaking change has been outlined in the release notes. Anyone else suffering from this issue, please comment here and I'll try to help to get your environment up and running.

@grabbou grabbou closed this as completed Oct 13, 2017
tindn pushed a commit to tindn/livestrong-native that referenced this issue Dec 21, 2017
renamed index.ios.js to index.js as this is the expected entry point. Additional information here facebook/react-native#16208
@facebook facebook locked as resolved and limited conversation to collaborators Oct 13, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

8 participants