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

[standards] Rewrite imports in YellowBox to use standard paths #24320

Closed
wants to merge 2 commits into from

Conversation

ide
Copy link
Contributor

@ide ide commented Apr 5, 2019

Summary

See #24316 for the motivation. This commit rewrites the imports in the YellowBox component.

Changelog

[General] [Changed] - Replaced Haste-style imports with standard path-style imports for YellowBox

Test Plan

Run RNTester and verify that it loads without any issues when it display a warning.

ide added 2 commits April 4, 2019 18:05
…imports

By adding the .android.js and .ios.js extensions to the respective .flowconfig files, Flow is able to find files that either:

- Are required using Haste
- Are required using standard paths and have a .js extension
- Are required using standard paths and have a .platform.js subextension

Test plan: After replacing some Haste-style requires with path-based requires, this change to the Flow config made Flow pass.
See facebook#24316 for the motivation. This commit rewrites the imports in the YellowBox component.

[General] [Changed] - Replaced Haste-style imports with standard path-style imports for YellowBox

Run RNTester and verify that it loads without any issues when it display a warning.
@ide ide requested a review from cpojer April 5, 2019 05:43
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Expo Partner: Expo Partner labels Apr 5, 2019
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

import type {Category} from 'YellowBoxCategory';
import type {Registry, Subscription, IgnorePattern} from 'YellowBoxRegistry';
import type {Category} from './Data/YellowBoxCategory';
import type {Registry, Subscription, IgnorePattern} from './Data/YellowBoxRegistry';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier/prettier: Replace Registry,·Subscription,·IgnorePattern with ⏎··Registry,⏎··Subscription,⏎··IgnorePattern,⏎

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase once I land the .flowconfig change.

@matthargett
Copy link
Contributor

@rigdern @empyrical @vincentriemer does this work for your out of tree platforms? It breaks in my tests because it misses the View.platform.js override (amongst others).

@vincentriemer
Copy link
Contributor

Yeah this will likely make it impossible for rndom (or any other out of tree platform) to build.

For a simplest case I'd think about how an out-of-tree platform would provide an implementation for Libraries/Utilities/Platform.<whatever>.js if it is imported as a relative path. That's a module that definitely needs to be overridden by the platform while others may theoretically be refactored to not need any overrides. There just isn't any mechanism that I know of right now to override relative paths other than explicit path aliasing (which imo would be a huge pain to maintain).

@rigdern
Copy link
Contributor

rigdern commented Apr 8, 2019

@acoates-ms see Matt's question above.

@acoates-ms
Copy link
Contributor

Yeah, this will be a pain to support in out of tree platforms. Before we can remove the haste modules within RN, we need to have a solution for how out of tree platforms can work. (Or we need to get all of the code in core to be completely cross platform with no platform checks...)

@815065046
Copy link

easy

@acoates-ms
Copy link
Contributor

@cpojer , did any write up get written on the proposed way to move out of tree platforms forward without haste?

I do think the new proposal will need some tooling / config work while the transition is taking place. Unless we get rid of haste completely within a single release, each platform will need to have its own hasteImpl, or the current one will need to know how to switch the haste project, rather than merging all the haste packages together as it currently does.

I've got one of our internal platforms over to the proposed mechanisms, but that platform doesn't have to worry quite so much about having all the tools / config work out of box since its internal only. I'd want to get that all sorted before moving react-native-windows over.

@ide
Copy link
Contributor Author

ide commented May 9, 2019

Closing this PR since it's out of date.

@ide ide closed this May 9, 2019
@ide ide deleted the standard-paths-yellowbox branch April 25, 2020 08:29
facebook-github-bot pushed a commit that referenced this pull request Apr 26, 2022
Summary:
This sync includes the following changes:
- **[bd4784c8f](facebook/react@bd4784c8f )**: Revert #24236 (Don't recreate the same fallback on the client if hydrating suspends) ([#24434](facebook/react#24434)) //<dan>//
- **[6d3b6d0f4](facebook/react@6d3b6d0f4 )**: forwardRef et al shouldn't affect if props reused ([#24421](facebook/react#24421)) //<Andrew Clark>//
- **[bd0813766](facebook/react@bd0813766 )**: Fix: useDeferredValue should reuse previous value ([#24413](facebook/react#24413)) //<Andrew Clark>//
- **[9ae80d6a2](facebook/react@9ae80d6a2 )**: Suppress hydration warnings when a preceding sibling suspends ([#24404](facebook/react#24404)) //<Josh Story>//
- **[0dc4e6663](facebook/react@0dc4e6663 )**: Land enableClientRenderFallbackOnHydrationMismatch ([#24410](facebook/react#24410)) //<Andrew Clark>//
- **[354772952](facebook/react@354772952 )**: Land enableSelectiveHydration flag ([#24406](facebook/react#24406)) //<Andrew Clark>//
- **[392808a1f](facebook/react@392808a1f )**: Land enableClientRenderFallbackOnTextMismatch flag ([#24405](facebook/react#24405)) //<Andrew Clark>//
- **[1e748b452](facebook/react@1e748b452 )**: Land enableLazyElements flag ([#24407](facebook/react#24407)) //<Andrew Clark>//
- **[4175f0593](facebook/react@4175f0593 )**: Temporarily feature flag numeric fallback for symbols ([#24401](facebook/react#24401)) //<Ricky>//
- **[a6d53f346](facebook/react@a6d53f346 )**: Revert "Clean up Selective Hydration / Event Replay flag ([#24156](facebook/react#24156))" ([#24402](facebook/react#24402)) //<Ricky>//
- **[ab9cdd34f](facebook/react@ab9cdd34f )**: Bugfix: In legacy mode, call suspended tree's unmount effects when it is deleted ([#24400](facebook/react#24400)) //<Andrew Clark>//
- **[168da8d55](facebook/react@168da8d55 )**: Fix typo that happened during rebasing //<Andrew Clark>//
- **[8bc527a4c](facebook/react@8bc527a4c )**: Bugfix: Fix race condition between interleaved and non-interleaved updates ([#24353](facebook/react#24353)) //<Andrew Clark>//
- **[f7cf077cc](facebook/react@f7cf077cc )**: [Transition Tracing] Add Offscreen Queue ([#24341](facebook/react#24341)) //<Luna Ruan>//
- **[4fc394bbe](facebook/react@4fc394bbe )**: Fix suspense fallback throttling ([#24253](facebook/react#24253)) //<sunderls>//
- **[80170a068](facebook/react@80170a068 )**: Match bundle.name and match upper case entry points ([#24346](facebook/react#24346)) //<Sebastian Markbåge>//
- **[fea6f8da6](facebook/react@fea6f8da6 )**: [Transition Tracing] Add transition to OffscreenState and pendingSuspenseBoundaries to RootState ([#24340](facebook/react#24340)) //<Luna Ruan>//
- **[8e2f9b086](facebook/react@8e2f9b086 )**: move passive flag ([#24339](facebook/react#24339)) //<Luna Ruan>//
- **[55a21ef7e](facebook/react@55a21ef7e )**: fix pushTransition for transition tracing ([#24338](facebook/react#24338)) //<Luna Ruan>//
- **[069d23bb7](facebook/react@069d23bb7 )**:  [eslint-plugin-exhaustive-deps] Fix exhaustive deps check for unstable vars ([#24343](facebook/react#24343)) //<Afzal Sayed>//
- **[4997515b9](facebook/react@4997515b9 )**: Point useSubscription to useSyncExternalStore shim ([#24289](facebook/react#24289)) //<dan>//
- **[01e2bff1d](facebook/react@01e2bff1d )**: Remove unnecessary check ([#24332](facebook/react#24332)) //<zhoulixiang>//
- **[d9a0f9e20](facebook/react@d9a0f9e20 )**: Delete create-subscription folder ([#24288](facebook/react#24288)) //<dan>//
- **[f993ffc51](facebook/react@f993ffc51 )**: Fix infinite update loop that happens when an unmemoized value is passed to useDeferredValue ([#24247](facebook/react#24247)) //<Andrew Clark>//
- **[fa5800226](facebook/react@fa5800226 )**: [Fizz] Pipeable Stream Perf ([#24291](facebook/react#24291)) //<Josh Story>//
- **[0568c0f8c](facebook/react@0568c0f8c )**: Replace zero with NoLanes for consistency in FiberLane ([#24327](facebook/react#24327)) //<Leo>//
- **[e0160d50c](facebook/react@e0160d50c )**: add transition tracing transitions stack ([#24321](facebook/react#24321)) //<Luna Ruan>//
- **[b0f13e5d3](facebook/react@b0f13e5d3 )**: add pendingPassiveTransitions ([#24320](facebook/react#24320)) //<Luna Ruan>//

Changelog:
[General][Changed] - React Native sync for revisions 60e63b9...bd4784c

jest_e2e[run_all_tests]

Reviewed By: kacieb

Differential Revision: D35899012

fbshipit-source-id: 86a885e336fca9f0efa80cd2b8ca040f2cb53853
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. p: Expo Partner: Expo Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet