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

Packager not handling .babelignore'd files due to sourcemap generation logic #13168

Closed
kevinold opened this issue Mar 27, 2017 · 8 comments
Closed
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@kevinold
Copy link

kevinold commented Mar 27, 2017

Description

I have a need to bundle a pre-optimized external lib with my RN application. Until RN 0.42 I had been using a .babelignore to prevent the packager from trying to optimize this file and choke.

It seems in 0.42 and higher I'm no longer allowed to ignore the file.

This issue has also been reported as #12071

I've chatted with @loganfsmyth from BabelJS and his comments were:

cool, I think this is a regression in react-native, their code in that stacktrace is not handling ignored files: https://github.com/facebook/react-native/blob/master/packager/transformer.js#L110, ast will be undefined if the file was ignored (edited)
I believe that code was added recently for their faster sourcemap generation logic

Reproduction Steps and Sample Code

A project running 0.43.0-rc4 which produces a more verbose error than 0.42 does is here:

https://github.com/kevinold/RNBabelIgnoreIssue

Upon running react-native run-ios the following error is produced:

error: bundling: TypeError: Cannot read property 'tokens' of undefined
    at new Generator (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/babel-generator/lib/index.js:51:21)
    at exports.default (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/babel-generator/lib/index.js:19:13)
    at Object.transform (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/react-native/packager/transformer.js:110:20)
    at transformCode (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/react-native/packager/src/JSTransformer/worker/worker.js:91:31)
    at exports.transformAndExtractDependencies (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/react-native/packager/src/JSTransformer/worker/worker.js:148:3)
    at handle (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/worker-farm/lib/child/index.js:41:8)
    at process.<anonymous> (/Users/kold/eval-projects/RNBabelIgnoreIssue/node_modules/worker-farm/lib/child/index.js:47:3)
    at emitTwo (events.js:106:13)
    at process.emit (events.js:194:7)
TransformError: /Users/kold/eval-projects/RNBabelIgnoreIssue/epublican-native-context-opt.js: Cannot read property 'tokens' of undefined
Bundling `index.ios.js`  99.0% (403/405), failed.

Solution

Handle babel ignored (in .babelrc ignore sections or .babelignore) files in the packager: https://github.com/facebook/react-native/blob/master/packager/transformer.js#L110

Additional Information

  • React Native version: Broken: 0.42-0.43.0-rc4, works on 0.41
  • Platform: iOS (I assume Android too)
  • Development Operating System: OS X El Capitan
@kevinold
Copy link
Author

@davidaurelio Hi David, see this issue. Adding you here as this might be related to your recent patch (0849f84#diff-4676ea0b3c55c65c3929aa993144f07f).

@kevinold
Copy link
Author

@davidaurelio Any chance you could look into this?

@loganfsmyth Would you be able to provide me with a bit of guidance on a patch for this issue if David is not able to?

@kevinold
Copy link
Author

As a workaround I am using haul to package my app with my ignored files in .babelrc instead of a .babelignore like so:

{
  "presets": ["react-native"],
  "ignore": [
    "my-ignored-lib.js"
  ]
}

kevinold pushed a commit to kevinold/react-native that referenced this issue Apr 27, 2017
facebook-github-bot pushed a commit to facebook/metro that referenced this issue May 6, 2017
Summary:
- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.

I have a need to bundle a pre-optimized external lib with my RN application. Until RN 0.42 I had been using a .babelignore to prevent the packager from trying to optimize this file and choke.

It seems in 0.42 and higher I'm no longer allowed to ignore the file.

This issue has also been reported as #12071

Details on the reasoning for this patch can be found in the issue I originally filed: facebook/react-native#13168

What existing problem does the pull request solve?

This PR restores the functionality with babel ignoring files that existed in 0.41 before this patch:

facebook/react-native@0849f84#diff-4676ea0b3c55c65c3929aa993144f07f

Here's a screenshot of this patch properly ignoring the file I referenced in facebook/react-native#13168 to be ignored.

![screen shot 2017-04-27 at 12 48 32 am](https://cloud.githubusercontent.com/assets/21967/25469653/524dbc0c-2ae3-11e7-81a6-faca2f4d21fe.png)

The patch relies on the `ignored` value of the call to `babel.transform` and if true returns the src in a object per instruction from loganfsmyth from BabelJS core team.

To test, add a file to the `ignore` array of a `.babelrc` file in a React Native project with this fork.

I was unable to locate a test file for the transformer.js

Fixes #12071, #13168
Closes facebook/react-native#13681

Differential Revision: D5017565

Pulled By: davidaurelio

fbshipit-source-id: 421f57b5ce192eedd46fae4285d8a741cb5f8e71
facebook-github-bot pushed a commit that referenced this issue May 8, 2017
Summary:
- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.

I have a need to bundle a pre-optimized external lib with my RN application. Until RN 0.42 I had been using a .babelignore to prevent the packager from trying to optimize this file and choke.

It seems in 0.42 and higher I'm no longer allowed to ignore the file.

This issue has also been reported as #12071

Details on the reasoning for this patch can be found in the issue I originally filed: #13168

What existing problem does the pull request solve?

This PR restores the functionality with babel ignoring files that existed in 0.41 before this patch:

0849f84#diff-4676ea0b3c55c65c3929aa993144f07f

Here's a screenshot of this patch properly ignoring the file I referenced in #13168 to be ignored.

![screen shot 2017-04-27 at 12 48 32 am](https://cloud.githubusercontent.com/assets/21967/25469653/524dbc0c-2ae3-11e7-81a6-faca2f4d21fe.png)

The patch relies on the `ignored` value of the call to `babel.transform` and if true returns the src in a object per instruction from loganfsmyth from BabelJS core team.

To test, add a file to the `ignore` array of a `.babelrc` file in a React Native project with this fork.

I was unable to locate a test file for the transformer.js

Fixes #12071, #13168
Closes #13681

Differential Revision: D5017565

Pulled By: davidaurelio

fbshipit-source-id: 421f57b5ce192eedd46fae4285d8a741cb5f8e71
makadaw pushed a commit to makadaw/react-native that referenced this issue May 9, 2017
Summary:
- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.

I have a need to bundle a pre-optimized external lib with my RN application. Until RN 0.42 I had been using a .babelignore to prevent the packager from trying to optimize this file and choke.

It seems in 0.42 and higher I'm no longer allowed to ignore the file.

This issue has also been reported as facebook#12071

Details on the reasoning for this patch can be found in the issue I originally filed: facebook#13168

What existing problem does the pull request solve?

This PR restores the functionality with babel ignoring files that existed in 0.41 before this patch:

facebook@0849f84#diff-4676ea0b3c55c65c3929aa993144f07f

Here's a screenshot of this patch properly ignoring the file I referenced in facebook#13168 to be ignored.

![screen shot 2017-04-27 at 12 48 32 am](https://cloud.githubusercontent.com/assets/21967/25469653/524dbc0c-2ae3-11e7-81a6-faca2f4d21fe.png)

The patch relies on the `ignored` value of the call to `babel.transform` and if true returns the src in a object per instruction from loganfsmyth from BabelJS core team.

To test, add a file to the `ignore` array of a `.babelrc` file in a React Native project with this fork.

I was unable to locate a test file for the transformer.js

Fixes facebook#12071, facebook#13168
Closes facebook#13681

Differential Revision: D5017565

Pulled By: davidaurelio

fbshipit-source-id: 421f57b5ce192eedd46fae4285d8a741cb5f8e71
@alexanderkjeldaas
Copy link

alexanderkjeldaas commented May 25, 2017

I'm having this issue, but with 0.45.0-rc.0, I'm getting out of memory. The behavior is not the same as with 0.40.0:

transform[stdout]:
transform[stdout]: <--- Last few GCs --->
transform[stdout]:
transform[stdout]: [24516:0x293f6c0]    67948 ms: Mark-sweep 1411.8 (1497.5) -> 1411.3 (1501.0) MB, 2285.6 / 0.0 ms  allocation failure GC in old space requested
transform[stdout]: [24516:0x293f6c0]    70247 ms: Mark-sweep 1411.3 (1501.0) -> 1411.3 (1470.0) MB, 2298.1 / 0.0 ms  last resort
transform[stdout]: [24516:0x293f6c0]    72527 ms: Mark-sweep 1411.3 (1470.0) -> 1411.3 (1470.0) MB, 2280.0 / 0.0 ms  last resort
transform[stdout]:
transform[stdout]:
transform[stdout]: <--- JS stacktrace --->
transform[stdout]:
transform[stdout]: ==== JS stack trace =========================================
transform[stdout]:
transform[stdout]: Security context: 0xca215ea66a1 <JS Object>
transform[stdout]:     2: parseExprOp [rn-code/node_modules/babylon/lib/index.js:~3416] [pc=0x4bbbf43ea8](this=0x684952922d9 <a Parser with map 0x2d43317b7999>,left=0x337d1acdc149 <a Node with map 0x2d43317c8351>,leftStartPos=1220248,leftStartLoc=0x337d1acdc0d9 <a Position with map 0x2d43317c56f9>,minPrec=-1,noIn=0x28c0c4c02311 <undefined>)
transform[stdout]:     3: parseMaybeAssign [rn-...
transform[stdout]:
transform[stderr]: FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
transform[stderr]:  1: node::Abort() [/usr/bin/nodejs]
transform[stderr]:  2: 0x134e91c [/usr/bin/nodejs]
transform[stderr]:  3:
transform[stderr]: v8::Utils::ReportOOMFailure(char const*, bool) [/usr/bin/nodejs]
transform[stderr]:  4:
transform[stderr]: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [/usr/bin/nodejs]
transform[stderr]:  5: v8::internal::Factory::NewUninitializedFixedArray(int) [/usr/bin/nodejs]
transform[stderr]:  6: 0xe1f0f5 [/usr/bin/nodejs]
transform[stderr]:  7:
transform[stderr]: v8::internal::Runtime_GrowArrayElements(int, v8::internal::Object**, v8::internal::Isolate*) [/usr/bin/nodejs]
transform[stderr]:  8: 0x4bbb7843a7

node v7.10.0

@hramos
Copy link
Contributor

hramos commented Jul 31, 2017

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

  • Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
  • If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
  • Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

@hramos hramos added the Icebox label Jul 31, 2017
@hramos hramos closed this as completed Jul 31, 2017
@alexanderkjeldaas
Copy link

I still have the same out-of-memory issue with 0.46.4

@GarimaMathur07
Copy link

@alexanderkjeldaas did you find any solution for Android?

@sujitpk-perennial
Copy link

@alexanderkjeldaas @GarimaMathur07 I am also getting JavaScript heap out of memory issue for large file. How you resolved this?

facebook-github-bot pushed a commit that referenced this issue Jul 24, 2018
Summary:
@public
This sync includes the following changes:
- **[ca0941fce](facebook/react@ca0941fce)**: Add regression test for Placeholder fallbacks with lifecycle methods (#13254) //<Andrew Clark>//
- **[a32c727f2](facebook/react@a32c727f2)**: Optimize readContext for Subsequent Reads of All Bits (#13248) //<Sebastian Markbåge>//
- **[2b509e2c8](facebook/react@2b509e2c8)**: [Experimental] API for reading context from within any render phase function (#13139) //<Andrew Clark>//
- **[5776fa3fc](facebook/react@5776fa3fc)**: Update www warning shim (#13244) //<Dan Abramov>//
- **[3d3506d37](facebook/react@3d3506d37)**: Include Modes in the component stack (#13240) //<Dan Abramov>//
- **[71b4e9990](facebook/react@71b4e9990)**: [react-test-renderer] Jest matchers for async tests (#13236) //<Andrew Clark>//
- **[2c560cb99](facebook/react@2c560cb99)**: Fix unwinding starting with a wrong Fiber on error in the complete phase (#13237) //<Dan Abramov>//
- **[ead08827d](facebook/react@ead08827d)**: Add more flexibility in testing errors in begin/complete phases (#13235) //<Dan Abramov>//
- **[e4e58343e](facebook/react@e4e58343e)**: Move unstable_yield to main export (#13232) //<Andrew Clark>//
- **[0e235bb8f](facebook/react@0e235bb8f)**: Removed unused state argument in unsubscribe method of <Subscription /> (#13233) //<Mateusz Burzyński>//
- **[236f60872](facebook/react@236f60872)**: Fail tests if toWarnDev() does not wrap warnings in array (#13227) //<Dan Abramov>//
- **[acbb4f93f](facebook/react@acbb4f93f)**: Remove the use of proxies for synthetic events in DEV (#13225) //<Dan Abramov>//
- **[171e0b7d4](facebook/react@171e0b7d4)**: Fix “no onChange handler” warning to fire on falsy values ("", 0, false) too (#12628) //<Nicole Levy>//
- **[606c30aa5](facebook/react@606c30aa5)**: fixed a typo in commentout in ReactFiberUnwindWork.js (#13172) //<Fumiya Shibusawa>//
- **[9f78913b2](facebook/react@9f78913b2)**: Update prettier (#13205) //<Johan Henriksson>//
- **[6d3e26288](facebook/react@6d3e26288)**: Remove unnecessary `typeof` checks (#13196) //<jddxf>//
- **[82c7ca4cc](facebook/react@82c7ca4cc)**: Add component stacks to some warnings (#13218) //<Dan Abramov>//
- **[21ac62c77](facebook/react@21ac62c77)**: Fix a portal unmounting crash for renderers with distinct Instance and Container (#13220) //<Thibault Malbranche>//
- **[d6a0626b3](facebook/react@d6a0626b3)**: Set current fiber during before-mutation traversal (#13219) //<Dan Abramov>//
- **[f9358c51c](facebook/react@f9358c51c)**: Change warning() to automatically inject the stack, and add warningWithoutStack() as opt-out (#13161) //<Dan Abramov>//
- **[467d13910](facebook/react@467d13910)**: Enforce presence or absence of component stack in tests (#13215) //<Dan Abramov>//
- **[43ffae2d1](facebook/react@43ffae2d1)**: Suspending inside a constructor outside of strict mode (#13200) //<Andrew Clark>//
- **[659a29cec](facebook/react@659a29cec)**: Reorganize how shared internals are accessed (#13201) //<Dan Abramov>//
- **[58f3b29d9](facebook/react@58f3b29d9)**: Added SSR/hydration tests for modes, forwardRef, and Profiler (#13195) //<Brian Vaughn>//
- **[1c89cb62f](facebook/react@1c89cb62f)**: Use ReactDebugCurrentFrame.getStackAddendum() in element validator (#13198) //<Dan Abramov>//
- **[e6076ecf4](facebook/react@e6076ecf4)**: Remove ad-hoc forks of getComponentName() and fix it (#13197) //<Dan Abramov>//
- **[32f6f258b](facebook/react@32f6f258b)**: Remove event simulation of onChange events (#13176) //<Philipp Spieß>//
- **[9ca37f843](facebook/react@9ca37f843)**: docs: update comments (#13043) //<Sen Yang>//
- **[f89f25f47](facebook/react@f89f25f47)**: Correct type of `ref` in forwardRef render() (#13100) //<Moti Zilberman>//
- **[7b99ceabe](facebook/react@7b99ceabe)**: Deprecate test utils mock component follow up (#13194) //<Brian Vaughn>//
- **[6ebc8f3c0](facebook/react@6ebc8f3c0)**: Add support for re-entrant SSR stacks (#13181) //<Dan Abramov>//
- **[d64d1ddb5](facebook/react@d64d1ddb5)**: Deprecate ReactTestUtils.mockComponent() (#13193) //<Brian Vaughn>//
- **[e79366d54](facebook/react@e79366d54)**: Link create-subscription doc to GH issue with de-opt explanation (#13187) //<Brian Vaughn>//
- **[1f32d3c6d](facebook/react@1f32d3c6d)**: Test renderer flushAll method verifies an array of expected yields (#13174) //<Brian Vaughn>//
- **[377e1a049](facebook/react@377e1a049)**: Add a test for SSR stack traces (#13180) //<Dan Abramov>//
- **[96d38d178](facebook/react@96d38d178)**: Fix concatenation of null to a warning message (#13166) //<Dan Abramov>//
- **[095dd5049](facebook/react@095dd5049)**: Add DEV warning if forwardRef function doesn't use the ref param (#13168) //<Brian Vaughn>//
- **[566259567](facebook/react@566259567)**: Refactor stack handling (no functional changes) (#13165) //<Dan Abramov>//
- **[ebbd22143](facebook/react@ebbd22143)**: Configure react-test-renderer as a secondary (#13164) //<Brandon Dail>//
- **[ddc91af79](facebook/react@ddc91af79)**: Decrease nested update limit from 1000 to 50 (#13163) //<Andrew Clark>//
- **[3596e40b3](facebook/react@3596e40b3)**: Fix nested update bug (#13160) //<Andrew Clark>//
- **[449f6ddd5](facebook/react@449f6ddd5)**: create a new FeatureFlags file for test renderer on www (#13159) //<Chang Yan>//
- **[f762b3abb](facebook/react@f762b3abb)**: Run react-dom SSR import test in jsdom-less environment (#13157) //<Dan Abramov>//
- **[6f6b560a6](facebook/react@6f6b560a6)**: Renamed selfBaseTime/treeBaseTime Fiber attributes to selfBaseDuration/treeBaseDuration (#13156) //<Brian Vaughn>//
- **[1386ccddd](facebook/react@1386ccddd)**: Fix ReferenceError when requestAnimationFrame isn't defined (#13152) //<Dan Abramov>//
- **[f5779bbc1](facebook/react@f5779bbc1)**: Run server rendering test on bundles (#13153) //<Dan Abramov>//
- **[9faf389e7](facebook/react@9faf389e7)**: Reset profiler timer correctly after errors (#13123) //<Brian Vaughn>//
- **[85fe4ddce](facebook/react@85fe4ddce)**: Fix - issue #12765 / the checked attribute is not initially set on the input (#13114) //<XuMM_12>//
- **[07fefe333](facebook/react@07fefe333)**: Drop handling for ms and O prefixes for CSS transition and animation events. (#13133) //<Rouven Weßling>//
- **[88d7ed8bf](facebook/react@88d7ed8bf)**: React.Timeout -> React.Placeholder (#13105) //<Andrew Clark>//
- **[f128fdea4](facebook/react@f128fdea4)**: Suspending outside of strict trees and async trees (#13098) //<Andrew Clark>//
- **[aa8266c4f](facebook/react@aa8266c4f)**: Prepare placeholders before timing out (#13092) //<Andrew Clark>//
- **[c039c16f2](facebook/react@c039c16f2)**: Fix this in a functional component for ShallowRenderer (#13144) //<Toru Kobayashi>//
- **[64e1921aa](facebook/react@64e1921aa)**: Fix Flow type that event target can be null (#13124) //<Sebastian Markbåge>//
- **[6d6de6011](facebook/react@6d6de6011)**: Add PROFILE bundles for www+DOM and fbsource+RN/RF (#13112) //<Brian Vaughn>//
- **[71a60ddb1](facebook/react@71a60ddb1)**: Add link to another article about React renderers //<Dan Abramov>//
- **[6a530e3ba](facebook/react@6a530e3ba)**: adding check for mousemove (#13090) //<Jason Williams>//
- **[c35a1e748](facebook/react@c35a1e748)**: Fix crash during server render in react 16.4.1. (#13088) //<Dustin Masters>//
- **[076bbeace](facebook/react@076bbeace)**: Fall back to 'setTimeout' when 'requestAnimationFrame' is not called (#13091) //<Flarnie Marchan>//
- **[da5c87bdf](facebook/react@da5c87bdf)**: Fixes children when using dangerouslySetInnerHtml in a selected <option> (#13078) //<Michael Ridgway>//
- **[a960d18bc](facebook/react@a960d18bc)**: eliminate unnecessary do-while loop in renderRoot() (#13087) //<Nathan Quarles>//
- **[5b3d17a5f](facebook/react@5b3d17a5f)**: setting a flag, so that the first movement will have the correct value (#13082) //<Jason Williams>//
- **[b0f60895f](facebook/react@b0f60895f)**: Automatically Profile roots when DevTools is present (#13058) //<Brian Vaughn>//
- **[ae8c6dd53](facebook/react@ae8c6dd53)**: remove some redundant lines (#13077) //<Nathan Quarles>//
- **[0fcf92d06](facebook/react@0fcf92d06)**: Add a link to custom renderer intro article //<Dan Abramov>//
- **[97af3e1f3](facebook/react@97af3e1f3)**:  Do not add additional work to a batch that is already rendering (#13072) //<Andrew Clark>//
- **[4fe6eec15](facebook/react@4fe6eec15)**: Always batch updates of like priority within the same event (#13071) //<Andrew Clark>//
- **[8e87c139b](facebook/react@8e87c139b)**: Remove transitive dependency on fbjs (#13075) //<Dan Abramov>//
- **[aeda7b745](facebook/react@aeda7b745)**: Remove fbjs dependency (#13069) //<Dan Abramov>//
- **[b1b3acbd6](facebook/react@b1b3acbd6)**: Inline fbjs/lib/emptyObject (#13055) //<Dan Abramov>//

Release Notes:
[GENERAL] [FEATURE] [React] - React sync for revisions ae14317...ca0941f

Reviewed By: bvaughn

Differential Revision: D8979192

fbshipit-source-id: 7a14d3b0a253a81d162d7f8c899e99cf6ac4fee4
@facebook facebook locked as resolved and limited conversation to collaborators Jul 31, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 31, 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

6 participants