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.

Sign up for GitHub

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

Jump to bottom

Fix regression in dynamic resolution for out-of-tree platforms #20662

Closed
wants to merge 15 commits into from
Closed

Fix regression in dynamic resolution for out-of-tree platforms #20662

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d314ef8
Initial changes to make platform resolution dynamic for out-of-tree p…
matthargett Aug 14, 2018
94425cc
Merge branch 'master' of github.com:matthargett/react-native into dyn…
matthargett Aug 15, 2018
a5190b7
Revert fixes for lint/prettier failures.
matthargett Aug 16, 2018
a51bffe
Merge branch 'master' of github.com:facebook/react-native into dynami…
matthargett Aug 16, 2018
771df00
Take Name.es6.js and Name.es7.js into account for regexp.
matthargett Aug 16, 2018
403930c
Revert prettier auto-fix.
matthargett Aug 16, 2018
1f7f362
Adjust comment and regexp. Note weird error when only two platforms a…
matthargett Aug 17, 2018
8dc4fe7
From @rozele: React Native overrides the settings for, e.g., , which…
matthargett Aug 22, 2018
9edfa98
Merge branch 'master' of github.com:facebook/react-native into dynami…
matthargett Aug 22, 2018
5207cd1
Remove hard-coded platform paths.
matthargett Aug 22, 2018
0e72049
hasteImpl will be overridden via metro configurations of out-of-tree …
matthargett Aug 23, 2018
3159e0c
Merge branch 'master' of github.com:facebook/react-native into dynami…
matthargett Aug 23, 2018
d366ea4
Update tests to remove out-of-tree platform specifics
matthargett Aug 23, 2018
9ce4e7a
Fix comment
matthargett Aug 23, 2018
5733fe1
My synthetic tests didn't capture real-world pathing. This should fix…
matthargett Aug 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest/__tests__/hasteImpl-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ it('returns the correct haste name for a RN library file', () => {
});

it('returns the correct haste name for a file with a platform suffix', () => {
for (const platform of ['android', 'ios', 'native', 'web', 'windows']) {
for (const platform of ['android', 'ios', 'native']) {
expect(
getHasteName(
getPath(
Expand Down
6 changes: 2 additions & 4 deletions jest/hasteImpl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ const path = require('path');

const ROOTS = [
path.resolve(__dirname, '..') + path.sep,
path.resolve(__dirname, '../../react-native-windows') + path.sep,
path.resolve(__dirname, '../../react-native-dom') + path.sep,
];

const BLACKLISTED_PATTERNS /*: Array<RegExp> */ = [
Expand All @@ -36,8 +34,8 @@ const NAME_REDUCERS /*: Array<[RegExp, string]> */ = [
[/^(?:.*[\\\/])?([a-zA-Z0-9$_.-]+)$/, '$1'],
// strip .js/.js.flow suffix
[/^(.*)\.js(\.flow)?$/, '$1'],
// strip .android/.ios/.native/.web suffix
[/^(.*)\.(android|ios|native|web|windows|dom)$/, '$1'],
// strip platform suffix
[/^(.*)\.(android|ios|native)$/, '$1'],
];

const haste = {
Expand Down
6 changes: 4 additions & 2 deletions local-cli/core/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,13 @@ const defaultConfig = {
hasteImplModulePath: require.resolve('../../jest/hasteImpl'),

getPlatforms(): Array<string> {
return ['ios', 'android', 'windows', 'web', 'dom'];
// if this is less than three items, AndroidTest stops resolving:
// Error: Unable to resolve module `ProgressBarAndroid` from `/Users/mhargett/workspace/react-native/ReactAndroid/src/androidTest/js/ProgressBarTestModule.js`: Module `ProgressBarAndroid` does not exist in the Haste module map
return ['ios', 'android', 'native', ...Object.keys(pluginPlatforms)];
},

getProvidesModuleNodeModules(): Array<string> {
return ['react-native', 'react-native-windows', 'react-native-dom'];
return ['react-native', ...plugins.platforms];
Copy link
Contributor

Choose a reason for hiding this comment

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

plugins.platforms is an array of strings like this:

[ 'react-native-foo/local-cli/platform.js' ]

Which it gets from an entry in react-native-foo's package.json like this:

  "rnpm": {
    "platform": "./local-cli/platform.js"
  }

But getProvidesModuleNodeModules() just wants entries like react-native-foo, so unless I did something wrong this doesn't seem to work.

BTW, I just discovered this repo:

https://github.com/react-native-community/discussions-and-proposals/issues

Perhaps a discussion on how renderer plugins could be implemented could be started there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Yes, that would be a good place to propose a new API, or discuss on the contributor's slack.

},
};

Expand Down