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

[Errors] Fix Red Box by fixing providesModule parsing #866

Closed
wants to merge 1 commit into from

Conversation

ide
Copy link
Contributor

@ide ide commented Apr 15, 2015

cc @amasad

An error occurred while trying to display the Red Box since loadSourceMap was not included in the JS
bundle. This is because node-haste was treating its docblock as a multiline directive which doesn't make sense for @providesModule.

In loadSourceMap.js's case, the directive's value was parsed as "loadSourceMap -- disabled flow due to mysterious validation errors --".

There are two fixes: add a newline under the @providesModule directive, and change the module ID code to look at only the first token of the directive. I opted for the latter so we avoid this class of bugs entirely and AFAIK it's nonsensical to have multiple @providesModule values anyway.

Test Plan: Run the packager, trigger an error in an app, see the red box now show up again.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 15, 2015
@amasad
Copy link
Contributor

amasad commented Apr 16, 2015

Great catch. Will pull soon.

An error occurred while trying to display the Red Box since loadSourceMap was not included in the JS bundle. This is because node-haste was treating its docblock as a multiline directive which doesn't make sense for `@providesModule`.

In loadSourceMap.js's case, the directive's value was parsed as "loadSourceMap -- disabled flow due to mysterious validation errors --".

There are two fixes: add a newline under the `@providesModule` directive, and change the module ID code to look at only the first token of the directive. I opted for the latter so we avoid this class of bugs entirely and AFAIK it's nonsensical to have multiple `@providesModule` values anyway.

Test Plan: Run the packager, trigger an error in an app, see the red box now show up again.
sahrens pushed a commit to sahrens/react-native that referenced this pull request Apr 22, 2015
Summary:
cc @amasad

An error occurred while trying to display the Red Box since loadSourceMap was not included in the JS
bundle. This is because node-haste was treating its docblock as a multiline directive which doesn't make sense for `@providesModule`.

In loadSourceMap.js's case, the directive's value was parsed as "loadSourceMap -- disabled flow due to mysterious validation errors --".

There are two fixes: add a newline under the `@providesModule` directive, and change the module ID code to look at only the first token of the directive. I opted for the latter so we avoid this class of bugs entirely and AFAIK it's nonsensical to have multiple `@providesModule` values anyway.

Closes facebook#866
Github Author: James Ide <ide@jameside.com>

Test Plan:  Run the packager, trigger an error in an app, see the red box now show up again.
@amasad amasad closed this Apr 23, 2015
@ide ide deleted the loadSourceMap-haste branch April 24, 2015 21:44
cpojer pushed a commit to facebook/metro that referenced this pull request Jan 26, 2017
Summary:
cc @amasad

An error occurred while trying to display the Red Box since loadSourceMap was not included in the JS
bundle. This is because node-haste was treating its docblock as a multiline directive which doesn't make sense for `@providesModule`.

In loadSourceMap.js's case, the directive's value was parsed as "loadSourceMap -- disabled flow due to mysterious validation errors --".

There are two fixes: add a newline under the `@providesModule` directive, and change the module ID code to look at only the first token of the directive. I opted for the latter so we avoid this class of bugs entirely and AFAIK it's nonsensical to have multiple `@providesModule` values anyway.

Closes facebook/react-native#866
Github Author: James Ide <ide@jameside.com>

Test Plan:  Run the packager, trigger an error in an app, see the red box now show up again.
ayushjainrksh pushed a commit to MLH-Fellowship/react-native that referenced this pull request Jul 2, 2020
mganandraj added a commit to mganandraj/react-native that referenced this pull request Nov 5, 2021
…ore publish (facebook#866)

* Some more changes for bringing up RN64 in devmain Android (facebook#861)

* Build & Packaging changes for bringing up RN64 in devmain

* Fixing gradle clean

* Disable gradle clean in PR builds

* Fix for submit button disappearing bug in comments (facebook#862)

* Add the missing android folder to files in package.json

* Fix for submit button disappear bug in comments

Co-authored-by: Mayuresh Gharpure <Mayuresh.Gharpure@microsoft.com>

* fix: ensure scripts always have Unix newlines (facebook#860)

* Fix for publishing from stable branches

* remove workspace info before publishing too

* Update rntester lock file when bumping package version

* Apply microsoft#875

* Allow newer cocoapod version

Co-authored-by: Anandraj <anandrag@microsoft.com>
Co-authored-by: Mayuresh Gharpure <gharpuremayuresh@gmail.com>
Co-authored-by: Mayuresh Gharpure <Mayuresh.Gharpure@microsoft.com>
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants