From 4d77c74c76565b14f61a6d7eb24b25befd396c1c Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 11 Oct 2017 13:18:40 -0700 Subject: [PATCH 01/32] Fix Analyze step on CI Summary: Recent changes that introduced Circle 2 (thanks ide and hramos for work on this) include special step for analyzing code. It takes PR number and processes the build. Unfortunately, that breaks all non-PR builds (including ones scheduled by me as a part of release step). This PR simply checks if such env variable is set and stops executing in case it's undefined. Also, I have updated the order of the tests so that most important (unit tests) are no longer shadowed by temporary eslint and flow breakage. The reason for this change is that flow has been broken for ~20 days which shadowed breakage in unit tests (addressed in my other PR). You can see build breaking before this change: https://circleci.com/gh/facebook/react-native/22391 And being green after: https://circleci.com/gh/facebook/react-native/22530 Closes https://github.com/facebook/react-native/pull/16302 Differential Revision: D6031348 Pulled By: hramos fbshipit-source-id: f1127a87faa872f413e9fcb780bdc1d5d587de2c --- .circleci/config.yml | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ace95d437146ac..cfc1eb70187d99 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -92,15 +92,19 @@ jobs: - checkout - run: npm install --no-package-lock - run: | + npm test -- --maxWorkers=2 npm run lint npm run flow -- check - npm test -- --maxWorkers=2 -# eslint +# eslint - doesn't run on non-PR builds - run: name: Analyze Code command: | - npm install github@0.2.4 - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js + if [ -n "$CIRCLE_PR_NUMBER" ]; then + npm install github@0.2.4 + cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js + else + echo "Skipping code analysis." + fi test-node-6: <<: *defaults @@ -110,9 +114,9 @@ jobs: - checkout - run: npm install - run: | + npm test -- --maxWorkers=2 npm run lint npm run flow -- check - npm test -- --maxWorkers=2 test-node-4: <<: *defaults @@ -122,10 +126,10 @@ jobs: - checkout - run: npm install - run: | + npm test -- --maxWorkers=2 npm run lint npm run flow -- check - npm test -- --maxWorkers=2 - + test-website: <<: *defaults docker: From 0ec04ed8ef11ee3b44833054466108872cfd3af7 Mon Sep 17 00:00:00 2001 From: Peter Ruibal Date: Wed, 11 Oct 2017 14:08:38 -0700 Subject: [PATCH 02/32] Remove redundant style field from ScrollView propTypes. Summary: We're spreading this in via `...ViewPropTypes` also. Having both confuses flow when you try to pass style (even though they're identical), when the types are defined via `React.ElementProps` Reviewed By: jingc Differential Revision: D6028659 fbshipit-source-id: 203e29682d34f1648a47d9ddbaef0c9630fbcb99 --- Libraries/Components/ScrollView/ScrollView.js | 1 - 1 file changed, 1 deletion(-) diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index 312aa75a94f49b..c3502df7294f82 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -331,7 +331,6 @@ const ScrollView = createReactClass({ * with `horizontal={true}`. */ stickyHeaderIndices: PropTypes.arrayOf(PropTypes.number), - style: StyleSheetPropType(ViewStylePropTypes), /** * When set, causes the scroll view to stop at multiples of the value of * `snapToInterval`. This can be used for paginating through children From 1f498010e8a10ab8b7f240988cbf892ef71c3154 Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 11 Oct 2017 14:51:58 -0700 Subject: [PATCH 03/32] Remove mockFs dependency Summary: This is first PR from the series I am going to be sending as a result of fixing 0.50-stable test suite. This one removes `mockFS` dependency that has been causing failures on Node 6.x container. Here's build before this change: https://circleci.com/gh/facebook/react-native/22529 Here's build after this change: https://circleci.com/gh/facebook/react-native/22538 (green) Note that the CI may be still red as there are other PRs to be addressed. You can see this in the wild on 0.50. Closes https://github.com/facebook/react-native/pull/16301 Differential Revision: D6031352 Pulled By: hramos fbshipit-source-id: 5c97ae6c87864c094e29e5d8987521071c67f5bd --- local-cli/{util => }/__mocks__/fs.js | 10 ++++- .../{util => }/__tests__/fs-mock-test.js | 0 local-cli/core/__fixtures__/android.js | 2 +- local-cli/core/__fixtures__/dependencies.js | 2 +- local-cli/core/__fixtures__/ios.js | 2 +- local-cli/core/__fixtures__/projects.js | 2 - .../android/findAndroidAppFolder.spec.js | 16 +++---- .../__tests__/android/findManifest.spec.js | 14 +++--- .../android/findPackageClassName.spec.js | 20 ++++----- .../android/getDependencyConfig.spec.js | 22 +++++----- .../android/getProjectConfig.spec.js | 20 ++++----- .../__tests__/android/readManifest.spec.js | 14 +++--- local-cli/core/__tests__/findAssets.spec.js | 16 +++---- .../core/__tests__/ios/findProject.spec.js | 44 +++++++++---------- .../__tests__/ios/getProjectConfig.spec.js | 16 +++---- package.json | 1 - 16 files changed, 95 insertions(+), 106 deletions(-) rename local-cli/{util => }/__mocks__/fs.js (97%) rename local-cli/{util => }/__tests__/fs-mock-test.js (100%) diff --git a/local-cli/util/__mocks__/fs.js b/local-cli/__mocks__/fs.js similarity index 97% rename from local-cli/util/__mocks__/fs.js rename to local-cli/__mocks__/fs.js index 0600c01120b24a..5aef1788542ce3 100644 --- a/local-cli/util/__mocks__/fs.js +++ b/local-cli/__mocks__/fs.js @@ -94,6 +94,9 @@ fs.readFileSync.mockImplementation(function(filepath, encoding) { if (isDirNode(node)) { throw new Error('Error readFileSync a dir: ' + filepath); } + if (Buffer.isBuffer(node) && typeof encoding !== 'undefined') { + return node.toString(); + } return node; }); @@ -133,7 +136,12 @@ function fsError(code, message) { } function isDirNode(node) { - return node && typeof node === 'object' && node.SYMLINK == null; + return ( + node && + typeof node === 'object' && + node.SYMLINK == null && + Buffer.isBuffer(node) === false + ); } function readlinkSync(filepath) { diff --git a/local-cli/util/__tests__/fs-mock-test.js b/local-cli/__tests__/fs-mock-test.js similarity index 100% rename from local-cli/util/__tests__/fs-mock-test.js rename to local-cli/__tests__/fs-mock-test.js diff --git a/local-cli/core/__fixtures__/android.js b/local-cli/core/__fixtures__/android.js index 2a94869e28254e..6e8c9c20d31108 100644 --- a/local-cli/core/__fixtures__/android.js +++ b/local-cli/core/__fixtures__/android.js @@ -1,4 +1,4 @@ -const fs = require('fs'); +const fs = require.requireActual('fs'); const path = require('path'); const manifest = fs.readFileSync(path.join(__dirname, './files/AndroidManifest.xml')); diff --git a/local-cli/core/__fixtures__/dependencies.js b/local-cli/core/__fixtures__/dependencies.js index ad0ce62529d81a..286064e8fe7d2f 100644 --- a/local-cli/core/__fixtures__/dependencies.js +++ b/local-cli/core/__fixtures__/dependencies.js @@ -1,4 +1,4 @@ -const fs = require('fs'); +const fs = require.requireActual('fs'); const path = require('path'); const android = require('./android'); diff --git a/local-cli/core/__fixtures__/ios.js b/local-cli/core/__fixtures__/ios.js index 50943559889555..f2d1ee7f5bd570 100644 --- a/local-cli/core/__fixtures__/ios.js +++ b/local-cli/core/__fixtures__/ios.js @@ -1,4 +1,4 @@ -const fs = require('fs'); +const fs = require.requireActual('fs'); const path = require('path'); exports.valid = { diff --git a/local-cli/core/__fixtures__/projects.js b/local-cli/core/__fixtures__/projects.js index 673aec6483f575..8b004f5b98f040 100644 --- a/local-cli/core/__fixtures__/projects.js +++ b/local-cli/core/__fixtures__/projects.js @@ -1,5 +1,3 @@ -const fs = require('fs'); -const path = require('path'); const android = require('./android'); const ios = require('./ios'); diff --git a/local-cli/core/__tests__/android/findAndroidAppFolder.spec.js b/local-cli/core/__tests__/android/findAndroidAppFolder.spec.js index 630809f0680b50..95d7e2ab25d575 100644 --- a/local-cli/core/__tests__/android/findAndroidAppFolder.spec.js +++ b/local-cli/core/__tests__/android/findAndroidAppFolder.spec.js @@ -11,13 +11,15 @@ 'use strict'; +jest.mock('fs'); + +const fs = require('fs'); const findAndroidAppFolder = require('../../android/findAndroidAppFolder'); -const mockFS = require('mock-fs'); const mocks = require('../../__fixtures__/android'); describe('android::findAndroidAppFolder', () => { beforeAll(() => { - mockFS({ + fs.__setMockFilesystem({ empty: {}, nested: { android: { @@ -31,15 +33,11 @@ describe('android::findAndroidAppFolder', () => { }); it('returns an android app folder if it exists in the given folder', () => { - expect(findAndroidAppFolder('flat')).toBe('android'); - expect(findAndroidAppFolder('nested')).toBe('android/app'); + expect(findAndroidAppFolder('/flat')).toBe('android'); + expect(findAndroidAppFolder('/nested')).toBe('android/app'); }); it('returns `null` if there is no android app folder', () => { - expect(findAndroidAppFolder('empty')).toBeNull(); - }); - - afterAll(() => { - mockFS.restore(); + expect(findAndroidAppFolder('/empty')).toBeNull(); }); }); diff --git a/local-cli/core/__tests__/android/findManifest.spec.js b/local-cli/core/__tests__/android/findManifest.spec.js index 032aa394f3f991..2c536eb3648ed6 100644 --- a/local-cli/core/__tests__/android/findManifest.spec.js +++ b/local-cli/core/__tests__/android/findManifest.spec.js @@ -11,13 +11,15 @@ 'use strict'; +jest.mock('fs'); + const findManifest = require('../../android/findManifest'); -const mockFS = require('mock-fs'); +const fs = require('fs'); const mocks = require('../../__fixtures__/android'); describe('android::findManifest', () => { beforeAll(() => { - mockFS({ + fs.__setMockFilesystem({ empty: {}, flat: { android: mocks.valid, @@ -26,14 +28,10 @@ describe('android::findManifest', () => { }); it('returns a manifest path if file exists in the folder', () => { - expect(typeof findManifest('flat')).toBe('string'); + expect(typeof findManifest('/flat')).toBe('string'); }); it('returns `null` if there is no manifest in the folder', () => { - expect(findManifest('empty')).toBeNull(); - }); - - afterAll(() => { - mockFS.restore(); + expect(findManifest('/empty')).toBeNull(); }); }); diff --git a/local-cli/core/__tests__/android/findPackageClassName.spec.js b/local-cli/core/__tests__/android/findPackageClassName.spec.js index a1cb7148161730..5b6df12c5e6c7a 100644 --- a/local-cli/core/__tests__/android/findPackageClassName.spec.js +++ b/local-cli/core/__tests__/android/findPackageClassName.spec.js @@ -11,13 +11,15 @@ 'use strict'; +jest.mock('fs'); + const findPackageClassName = require('../../android/findPackageClassName'); -const mockFS = require('mock-fs'); +const fs = require('fs'); const mocks = require('../../__fixtures__/android'); describe('android::findPackageClassName', () => { beforeAll(() => { - mockFS({ + fs.__setMockFilesystem({ empty: {}, flatJava: { android: mocks.valid, @@ -29,22 +31,20 @@ describe('android::findPackageClassName', () => { }); it('returns manifest content if file exists in the folder', () => { - expect(typeof findPackageClassName('flatJava')).toBe('string'); + expect(typeof findPackageClassName('/flatJava')).toBe('string'); }); it('returns the name of the java class implementing ReactPackage', () => { - expect(findPackageClassName('flatJava')).toBe('SomeExampleJavaPackage'); + expect(findPackageClassName('/flatJava')).toBe('SomeExampleJavaPackage'); }); it('returns the name of the kotlin class implementing ReactPackage', () => { - expect(findPackageClassName('flatKotlin')).toBe('SomeExampleKotlinPackage'); + expect(findPackageClassName('/flatKotlin')).toBe( + 'SomeExampleKotlinPackage', + ); }); it('returns `null` if there are no matches', () => { - expect(findPackageClassName('empty')).toBeNull(); - }); - - afterAll(() => { - mockFS.restore(); + expect(findPackageClassName('/empty')).toBeNull(); }); }); diff --git a/local-cli/core/__tests__/android/getDependencyConfig.spec.js b/local-cli/core/__tests__/android/getDependencyConfig.spec.js index 1923327dfd08cf..7320bac47a969f 100644 --- a/local-cli/core/__tests__/android/getDependencyConfig.spec.js +++ b/local-cli/core/__tests__/android/getDependencyConfig.spec.js @@ -11,15 +11,17 @@ 'use strict'; +jest.mock('fs'); + const getDependencyConfig = require('../../android').dependencyConfig; -const mockFS = require('mock-fs'); +const fs = require('fs'); const mocks = require('../../__fixtures__/android'); const userConfig = {}; describe('android::getDependencyConfig', () => { beforeAll(() => { - mockFS({ + fs.__setMockFilesystem({ empty: {}, nested: { android: { @@ -38,27 +40,23 @@ describe('android::getDependencyConfig', () => { }); it('returns an object with android project configuration', () => { - expect(getDependencyConfig('nested', userConfig)).not.toBeNull(); - expect(typeof getDependencyConfig('nested', userConfig)).toBe('object'); + expect(getDependencyConfig('/nested', userConfig)).not.toBeNull(); + expect(typeof getDependencyConfig('/nested', userConfig)).toBe('object'); }); it('returns `null` if manifest file has not been found', () => { - expect(getDependencyConfig('empty', userConfig)).toBeNull(); + expect(getDependencyConfig('/empty', userConfig)).toBeNull(); }); it('returns `null` if android project was not found', () => { - expect(getDependencyConfig('empty', userConfig)).toBeNull(); + expect(getDependencyConfig('/empty', userConfig)).toBeNull(); }); it('returns `null` if android project does not contain ReactPackage', () => { - expect(getDependencyConfig('noPackage', userConfig)).toBeNull(); + expect(getDependencyConfig('/noPackage', userConfig)).toBeNull(); }); it('returns `null` if it cannot find a packageClassName', () => { - expect(getDependencyConfig('corrupted', userConfig)).toBeNull(); - }); - - afterAll(() => { - mockFS.restore(); + expect(getDependencyConfig('/corrupted', userConfig)).toBeNull(); }); }); diff --git a/local-cli/core/__tests__/android/getProjectConfig.spec.js b/local-cli/core/__tests__/android/getProjectConfig.spec.js index a6395244fb99bf..a71d470009cc6d 100644 --- a/local-cli/core/__tests__/android/getProjectConfig.spec.js +++ b/local-cli/core/__tests__/android/getProjectConfig.spec.js @@ -11,13 +11,15 @@ 'use strict'; +jest.mock('fs'); + const getProjectConfig = require('../../android').projectConfig; -const mockFS = require('mock-fs'); +const fs = require('fs'); const mocks = require('../../__fixtures__/android'); describe('android::getProjectConfig', () => { beforeAll(() => { - mockFS({ + fs.__setMockFilesystem({ empty: {}, nested: { android: { @@ -38,7 +40,7 @@ describe('android::getProjectConfig', () => { it("returns `null` if manifest file hasn't been found", () => { const userConfig = {}; - const folder = 'noManifest'; + const folder = '/noManifest'; expect(getProjectConfig(folder, userConfig)).toBeNull(); }); @@ -46,7 +48,7 @@ describe('android::getProjectConfig', () => { describe('returns an object with android project configuration for', () => { it('nested structure', () => { const userConfig = {}; - const folder = 'nested'; + const folder = '/nested'; expect(getProjectConfig(folder, userConfig)).not.toBeNull(); expect(typeof getProjectConfig(folder, userConfig)).toBe('object'); @@ -54,7 +56,7 @@ describe('android::getProjectConfig', () => { it('flat structure', () => { const userConfig = {}; - const folder = 'flat'; + const folder = '/flat'; expect(getProjectConfig(folder, userConfig)).not.toBeNull(); expect(typeof getProjectConfig(folder, userConfig)).toBe('object'); @@ -64,7 +66,7 @@ describe('android::getProjectConfig', () => { const userConfig = { manifestPath: 'src/main/AndroidManifest.xml', }; - const folder = 'multiple'; + const folder = '/multiple'; expect(getProjectConfig(folder, userConfig)).not.toBeNull(); expect(typeof getProjectConfig(folder, userConfig)).toBe('object'); @@ -73,12 +75,8 @@ describe('android::getProjectConfig', () => { it('should return `null` if android project was not found', () => { const userConfig = {}; - const folder = 'empty'; + const folder = '/empty'; expect(getProjectConfig(folder, userConfig)).toBeNull(); }); - - afterAll(() => { - mockFS.restore(); - }); }); diff --git a/local-cli/core/__tests__/android/readManifest.spec.js b/local-cli/core/__tests__/android/readManifest.spec.js index dd88c960ec8226..631acf13bca987 100644 --- a/local-cli/core/__tests__/android/readManifest.spec.js +++ b/local-cli/core/__tests__/android/readManifest.spec.js @@ -11,14 +11,16 @@ 'use strict'; +jest.mock('fs'); + const findManifest = require('../../android/findManifest'); const readManifest = require('../../android/readManifest'); -const mockFS = require('mock-fs'); +const fs = require('fs'); const mocks = require('../../__fixtures__/android'); describe('android::readManifest', () => { beforeAll(() => { - mockFS({ + fs.__setMockFilesystem({ empty: {}, nested: { android: { @@ -29,19 +31,15 @@ describe('android::readManifest', () => { }); it('returns manifest content if file exists in the folder', () => { - const manifestPath = findManifest('nested'); + const manifestPath = findManifest('/nested'); expect(readManifest(manifestPath)).not.toBeNull(); expect(typeof readManifest(manifestPath)).toBe('object'); }); it('throws an error if there is no manifest in the folder', () => { - const fakeManifestPath = findManifest('empty'); + const fakeManifestPath = findManifest('/empty'); expect(() => { readManifest(fakeManifestPath); }).toThrow(); }); - - afterAll(() => { - mockFS.restore(); - }); }); diff --git a/local-cli/core/__tests__/findAssets.spec.js b/local-cli/core/__tests__/findAssets.spec.js index 484d0c09b33e05..ca7c0771279429 100644 --- a/local-cli/core/__tests__/findAssets.spec.js +++ b/local-cli/core/__tests__/findAssets.spec.js @@ -11,24 +11,26 @@ 'use strict'; +jest.mock('fs'); + const findAssets = require('../findAssets'); const dependencies = require('../__fixtures__/dependencies'); -const mockFs = require('mock-fs'); +const fs = require('fs'); describe('findAssets', () => { beforeEach(() => { - mockFs({testDir: dependencies.withAssets}); + fs.__setMockFilesystem({testDir: dependencies.withAssets}); }); it('returns an array of all files in given folders', () => { - const assets = findAssets('testDir', ['fonts', 'images']); + const assets = findAssets('/testDir', ['fonts', 'images']); expect(Array.isArray(assets)).toBeTruthy(); expect(assets).toHaveLength(3); }); it('prepends assets paths with the folder path', () => { - const assets = findAssets('testDir', ['fonts', 'images']); + const assets = findAssets('/testDir', ['fonts', 'images']); assets.forEach(assetPath => { expect(assetPath).toContain('testDir'); @@ -36,10 +38,6 @@ describe('findAssets', () => { }); it('returns an empty array if given assets are null', () => { - expect(findAssets('testDir', null)).toHaveLength(0); - }); - - afterEach(() => { - mockFs.restore(); + expect(findAssets('/testDir', null)).toHaveLength(0); }); }); diff --git a/local-cli/core/__tests__/ios/findProject.spec.js b/local-cli/core/__tests__/ios/findProject.spec.js index 1000f9733fb3b7..6492415d550c21 100644 --- a/local-cli/core/__tests__/ios/findProject.spec.js +++ b/local-cli/core/__tests__/ios/findProject.spec.js @@ -11,49 +11,51 @@ 'use strict'; +jest.mock('fs'); + const findProject = require('../../ios/findProject'); -const mockFS = require('mock-fs'); +const fs = require('fs'); const projects = require('../../__fixtures__/projects'); const ios = require('../../__fixtures__/ios'); describe('ios::findProject', () => { it('returns path to xcodeproj if found', () => { - mockFS(projects.flat); - expect(findProject('')).not.toBeNull(); + fs.__setMockFilesystem(projects.flat); + expect(findProject('/')).not.toBeNull(); }); it('returns null if there are no projects', () => { - mockFS({testDir: projects}); - expect(findProject('')).toBeNull(); + fs.__setMockFilesystem({testDir: projects}); + expect(findProject('/')).toBeNull(); }); it('returns ios project regardless of its name', () => { - mockFS({ios: ios.validTestName}); - expect(findProject('')).not.toBeNull(); + fs.__setMockFilesystem({ios: ios.validTestName}); + expect(findProject('/')).not.toBeNull(); }); it('ignores node_modules', () => { - mockFS({node_modules: projects.flat}); - expect(findProject('')).toBeNull(); + fs.__setMockFilesystem({node_modules: projects.flat}); + expect(findProject('/')).toBeNull(); }); it('ignores Pods', () => { - mockFS({Pods: projects.flat}); - expect(findProject('')).toBeNull(); + fs.__setMockFilesystem({Pods: projects.flat}); + expect(findProject('/')).toBeNull(); }); it('ignores Pods inside `ios` folder', () => { - mockFS({ + fs.__setMockFilesystem({ ios: { Pods: projects.flat, DemoApp: projects.flat.ios, }, }); - expect(findProject('')).toBe('ios/DemoApp/demoProject.xcodeproj'); + expect(findProject('/')).toBe('ios/DemoApp/demoProject.xcodeproj'); }); it('ignores xcodeproj from example folders', () => { - mockFS({ + fs.__setMockFilesystem({ examples: projects.flat, Examples: projects.flat, example: projects.flat, @@ -61,11 +63,11 @@ describe('ios::findProject', () => { Zpp: projects.flat, }); - expect(findProject('').toLowerCase()).not.toContain('example'); + expect(findProject('/').toLowerCase()).not.toContain('example'); }); it('ignores xcodeproj from sample folders', () => { - mockFS({ + fs.__setMockFilesystem({ samples: projects.flat, Samples: projects.flat, sample: projects.flat, @@ -73,11 +75,11 @@ describe('ios::findProject', () => { Zpp: projects.flat, }); - expect(findProject('').toLowerCase()).not.toContain('sample'); + expect(findProject('/').toLowerCase()).not.toContain('sample'); }); it('ignores xcodeproj from test folders at any level', () => { - mockFS({ + fs.__setMockFilesystem({ test: projects.flat, IntegrationTests: projects.flat, tests: projects.flat, @@ -87,10 +89,6 @@ describe('ios::findProject', () => { }, }); - expect(findProject('').toLowerCase()).not.toContain('test'); - }); - - afterEach(() => { - mockFS.restore(); + expect(findProject('/').toLowerCase()).not.toContain('test'); }); }); diff --git a/local-cli/core/__tests__/ios/getProjectConfig.spec.js b/local-cli/core/__tests__/ios/getProjectConfig.spec.js index 7b77f845201fab..d6d21d7ed9956f 100644 --- a/local-cli/core/__tests__/ios/getProjectConfig.spec.js +++ b/local-cli/core/__tests__/ios/getProjectConfig.spec.js @@ -11,32 +11,34 @@ 'use strict'; +jest.mock('fs'); + const getProjectConfig = require('../../ios').projectConfig; -const mockFS = require('mock-fs'); +const fs = require('fs'); const projects = require('../../__fixtures__/projects'); describe('ios::getProjectConfig', () => { const userConfig = {}; beforeEach(() => { - mockFS({testDir: projects}); + fs.__setMockFilesystem({testDir: projects}); }); it('returns an object with ios project configuration', () => { - const folder = 'testDir/nested'; + const folder = '/testDir/nested'; expect(getProjectConfig(folder, userConfig)).not.toBeNull(); expect(typeof getProjectConfig(folder, userConfig)).toBe('object'); }); it('returns `null` if ios project was not found', () => { - const folder = 'testDir/empty'; + const folder = '/testDir/empty'; expect(getProjectConfig(folder, userConfig)).toBeNull(); }); it('returns normalized shared library names', () => { - const projectConfig = getProjectConfig('testDir/nested', { + const projectConfig = getProjectConfig('/testDir/nested', { sharedLibraries: ['libc++', 'libz.tbd', 'HealthKit', 'HomeKit.framework'], }); @@ -47,8 +49,4 @@ describe('ios::getProjectConfig', () => { 'HomeKit.framework', ]); }); - - afterEach(() => { - mockFS.restore(); - }); }); diff --git a/package.json b/package.json index 6f70f0165131fe..90765a3b210e54 100644 --- a/package.json +++ b/package.json @@ -200,7 +200,6 @@ "eslint-plugin-react": "^7.2.1", "flow-bin": "^0.56.0", "jest": "^21", - "mock-fs": "^4.4.1", "prettier": "1.7.0", "react": "16.0.0", "react-test-renderer": "16.0.0", From cc86d12175cad69526b35211c5fdb24d23148e93 Mon Sep 17 00:00:00 2001 From: Alex Kotliarskyi Date: Wed, 11 Oct 2017 14:53:51 -0700 Subject: [PATCH 04/32] Android: put all non-drawable resources to `res/raw` Summary: When we built packager asset system we were mostly concerned about images. However, this system can also be used to work with videos, animations and other binary resources. The code that sorts assets into Android resource folders currently just shoves all non-drawable resources under `drawable-mdpi`, which is not ideal. Talking to Android experts on the team, `raw` seems like a much better place for other resources. Reviewed By: jeanlauliac Differential Revision: D6026633 fbshipit-source-id: cc2199f60da411ea432972a02f52c459ff5c490a --- Libraries/Image/AssetSourceResolver.js | 2 +- .../__tests__/getAssetDestPathAndroid-test.js | 12 ++++++++++++ local-cli/bundle/assetPathUtils.js | 18 ++++++++++++++++-- local-cli/bundle/getAssetDestPathAndroid.js | 2 +- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/Libraries/Image/AssetSourceResolver.js b/Libraries/Image/AssetSourceResolver.js index 53d80be0ce7a09..0371dbad642c8c 100644 --- a/Libraries/Image/AssetSourceResolver.js +++ b/Libraries/Image/AssetSourceResolver.js @@ -42,7 +42,7 @@ function getScaledAssetPath(asset): string { */ function getAssetPathInDrawableFolder(asset): string { var scale = AssetSourceResolver.pickScale(asset.scales, PixelRatio.get()); - var drawbleFolder = assetPathUtils.getAndroidDrawableFolderName(asset, scale); + var drawbleFolder = assetPathUtils.getAndroidResourceFolderName(asset, scale); var fileName = assetPathUtils.getAndroidResourceIdentifier(asset); return drawbleFolder + '/' + fileName + '.' + asset.type; } diff --git a/local-cli/bundle/__tests__/getAssetDestPathAndroid-test.js b/local-cli/bundle/__tests__/getAssetDestPathAndroid-test.js index 3f58299549cc05..410fb380ec4441 100644 --- a/local-cli/bundle/__tests__/getAssetDestPathAndroid-test.js +++ b/local-cli/bundle/__tests__/getAssetDestPathAndroid-test.js @@ -58,4 +58,16 @@ describe('getAssetDestPathAndroid', () => { getAssetDestPathAndroid(asset, 1).startsWith('assets_') ).toBeFalsy(); }); + + it('should put non-drawable resources to `raw/`', () => { + const asset = { + name: 'video', + type: 'mp4', + httpServerLocation: '/assets/app/test', + }; + + expect(getAssetDestPathAndroid(asset, 1)).toBe( + 'raw/app_test_video.mp4' + ); + }); }); diff --git a/local-cli/bundle/assetPathUtils.js b/local-cli/bundle/assetPathUtils.js index 26f44d39eb32a6..cc67d949ca69ce 100644 --- a/local-cli/bundle/assetPathUtils.js +++ b/local-cli/bundle/assetPathUtils.js @@ -29,7 +29,21 @@ function getAndroidAssetSuffix(scale: number): string { throw new Error('no such scale'); } -function getAndroidDrawableFolderName(asset: PackagerAsset, scale: number) { +// See https://developer.android.com/guide/topics/resources/drawable-resource.html +const drawableFileTypes = new Set([ + 'gif', + 'jpeg', + 'jpg', + 'png', + 'svg', + 'webp', + 'xml', +]); + +function getAndroidResourceFolderName(asset: PackagerAsset, scale: number) { + if (!drawableFileTypes.has(asset.type)) { + return 'raw'; + } var suffix = getAndroidAssetSuffix(scale); if (!suffix) { throw new Error( @@ -60,7 +74,7 @@ function getBasePath(asset: PackagerAsset) { module.exports = { getAndroidAssetSuffix: getAndroidAssetSuffix, - getAndroidDrawableFolderName: getAndroidDrawableFolderName, + getAndroidResourceFolderName: getAndroidResourceFolderName, getAndroidResourceIdentifier: getAndroidResourceIdentifier, getBasePath: getBasePath }; diff --git a/local-cli/bundle/getAssetDestPathAndroid.js b/local-cli/bundle/getAssetDestPathAndroid.js index c9f7e0192c7c69..c267aed8ba446b 100644 --- a/local-cli/bundle/getAssetDestPathAndroid.js +++ b/local-cli/bundle/getAssetDestPathAndroid.js @@ -17,7 +17,7 @@ const path = require('path'); import type {PackagerAsset} from '../../Libraries/Image/AssetRegistry'; function getAssetDestPathAndroid(asset: PackagerAsset, scale: number): string { - const androidFolder = assetPathUtils.getAndroidDrawableFolderName(asset, scale); + const androidFolder = assetPathUtils.getAndroidResourceFolderName(asset, scale); const fileName = assetPathUtils.getAndroidResourceIdentifier(asset); return path.join(androidFolder, fileName + '.' + asset.type); } From b2eb7fd77b9c553de37916cd63d95a54def96f31 Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Wed, 11 Oct 2017 19:34:06 -0700 Subject: [PATCH 05/32] Fix broken releases script Summary: Fails on my machine due to fact that `replace` returns an instance of a String, rather than an instance of ShellString (that includes `to` on its prototype). Solution is to use an explicit `writeFileSync`. You can see that change in the wild on 0.50-stable branch. CC janicduplessis (edit by hramos) Closes https://github.com/facebook/react-native/pull/16303 Differential Revision: D6031331 Pulled By: hramos fbshipit-source-id: 41c583d53df75bea1a55fa19174d912e414209c0 --- scripts/bump-oss-version.js | 53 ++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/scripts/bump-oss-version.js b/scripts/bump-oss-version.js index 3c91e6bee025f7..b8fe500a621383 100755 --- a/scripts/bump-oss-version.js +++ b/scripts/bump-oss-version.js @@ -15,7 +15,7 @@ * After changing the files it makes a commit and tags it. * All you have to do is push changes to remote and CI will make a new build. */ - +const fs = require('fs'); const { cat, echo, @@ -58,30 +58,39 @@ if (!match) { } let [, major, minor, patch, prerelease] = match; -cat('scripts/versiontemplates/ReactNativeVersion.java.template') - .replace('${major}', major) - .replace('${minor}', minor) - .replace('${patch}', patch) - .replace('${prerelease}', prerelease !== undefined ? `"${prerelease}"` : 'null') - .to('ReactAndroid/src/main/java/com/facebook/react/modules/systeminfo/ReactNativeVersion.java'); - -cat('scripts/versiontemplates/RCTVersion.h.template') - .replace('${major}', `@(${major})`) - .replace('${minor}', `@(${minor})`) - .replace('${patch}', `@(${patch})`) - .replace('${prerelease}', prerelease !== undefined ? `@"${prerelease}"` : '[NSNull null]') - .to('React/Base/RCTVersion.h'); - -cat('scripts/versiontemplates/ReactNativeVersion.js.template') - .replace('${major}', major) - .replace('${minor}', minor) - .replace('${patch}', patch) - .replace('${prerelease}', prerelease !== undefined ? `'${prerelease}'` : 'null') - .to('Libraries/Core/ReactNativeVersion.js'); +fs.writeFileSync( + 'ReactAndroid/src/main/java/com/facebook/react/modules/systeminfo/ReactNativeVersion.java', + cat('scripts/versiontemplates/ReactNativeVersion.java.template') + .replace('${major}', major) + .replace('${minor}', minor) + .replace('${patch}', patch) + .replace('${prerelease}', prerelease !== undefined ? `"${prerelease}"` : 'null'), + 'utf-8' +); + +fs.writeFileSync( + 'React/Base/RCTVersion.h', + cat('scripts/versiontemplates/RCTVersion.h.template') + .replace('${major}', `@(${major})`) + .replace('${minor}', `@(${minor})`) + .replace('${patch}', `@(${patch})`) + .replace('${prerelease}', prerelease !== undefined ? `@"${prerelease}"` : '[NSNull null]'), + 'utf-8' +); + +fs.writeFileSync( + 'Libraries/Core/ReactNativeVersion.js', + cat('scripts/versiontemplates/ReactNativeVersion.js.template') + .replace('${major}', major) + .replace('${minor}', minor) + .replace('${patch}', patch) + .replace('${prerelease}', prerelease !== undefined ? `'${prerelease}'` : 'null'), + 'utf-8' +); let packageJson = JSON.parse(cat('package.json')); packageJson.version = version; -JSON.stringify(packageJson, null, 2).to('package.json'); +fs.writeFileSync('package.json', JSON.stringify(packageJson, null, 2), 'utf-8'); // - change ReactAndroid/gradle.properties if (sed('-i', /^VERSION_NAME=.*/, `VERSION_NAME=${version}`, 'ReactAndroid/gradle.properties').code) { From acd9a29d94f145ae5b1c755ace7bb71964e363c6 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Thu, 12 Oct 2017 03:11:59 -0700 Subject: [PATCH 06/32] js1 metro-bundler: add script to publish new version Reviewed By: davidaurelio Differential Revision: D6019220 fbshipit-source-id: 97bb53323b29609e192f0e4f4d79b6be6440b98e --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 90765a3b210e54..8d03b4366620a7 100644 --- a/package.json +++ b/package.json @@ -162,7 +162,7 @@ "graceful-fs": "^4.1.3", "inquirer": "^3.0.6", "lodash": "^4.16.6", - "metro-bundler": "^0.19.0", + "metro-bundler": "^0.20.0", "mime": "^1.3.4", "minimist": "^1.2.0", "mkdirp": "^0.5.1", From 0da1738e9b7fd848a728397bb08c094eb4da56c0 Mon Sep 17 00:00:00 2001 From: Dan Caspi Date: Thu, 12 Oct 2017 08:08:49 -0700 Subject: [PATCH 07/32] Final fixes Reviewed By: amnn Differential Revision: D5932093 fbshipit-source-id: f52a0bb3552af5ad003c051dc32dff0bfabb43ac --- ReactCommon/cxxreact/JSCExecutor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 486961001ab1a0..3dfe2db47257d4 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -651,7 +651,7 @@ namespace facebook { void JSCExecutor::loadModule(uint32_t bundleId, uint32_t moduleId) { auto module = m_bundleRegistry->getModule(bundleId, moduleId); auto sourceUrl = String::createExpectingAscii(m_context, module.name); - auto source = String::createExpectingAscii(m_context, module.code); + auto source = adoptString(std::unique_ptr(new JSBigStdString(module.code))); evaluateScript(m_context, source, sourceUrl); } From 38354d94f11009a22871fa0c072a425a2a1584a9 Mon Sep 17 00:00:00 2001 From: Tim Wang Date: Thu, 12 Oct 2017 08:43:22 -0700 Subject: [PATCH 08/32] Add timwangdev to GitHub Issue task force Summary: I've submit few PRs for bug fix in recent releases and I've been working actively on some commercial react-native projects. I'd like to contribute the project with issues management and PR reviews. Closes https://github.com/facebook/react-native/pull/16328 Differential Revision: D6040721 Pulled By: hramos fbshipit-source-id: 61347fe53112c5ed89c6d58d2c7fec341db974be --- bots/IssueCommands.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bots/IssueCommands.txt b/bots/IssueCommands.txt index aae4d34433da0a..dc90f4f87d7e59 100644 --- a/bots/IssueCommands.txt +++ b/bots/IssueCommands.txt @@ -1,4 +1,4 @@ -React Native GitHub Issue Task Force: AndrewJack, astreet, bestander, brentvatne, browniefed, cancan101, charpeni, chirag04, christopherdro, corbt, cosmith, damusnet, DanielMSchmidt, davidaurelio, dmmiller, dsibiski, foghina, frantic, gantman, geirman, grabbou, gre, ide, janicduplessis, javache, jaygarcia, jsierles, kmagiera, knowbody, kmagiera, Kureev, lelandrichardson, martinbigio, melihmucuk, mkonicek, ncuillery, radko93, react-native-bot, rigdern, rmevans9, rt2zz, ryankask, satya164, skevy, tabrindle, vjeux +React Native GitHub Issue Task Force: AndrewJack, astreet, bestander, brentvatne, browniefed, cancan101, charpeni, chirag04, christopherdro, corbt, cosmith, damusnet, DanielMSchmidt, davidaurelio, dmmiller, dsibiski, foghina, frantic, gantman, geirman, grabbou, gre, ide, janicduplessis, javache, jaygarcia, jsierles, kmagiera, knowbody, kmagiera, Kureev, lelandrichardson, martinbigio, melihmucuk, mkonicek, ncuillery, radko93, react-native-bot, rigdern, rmevans9, rt2zz, ryankask, satya164, skevy, tabrindle, timwangdev, vjeux @facebook-github-bot answered comment Closing this issue as {author} says the question asked has been answered. From 73eb6c2481d7d2dd16a2ae4341ff9eb8477db688 Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Thu, 12 Oct 2017 08:51:31 -0700 Subject: [PATCH 09/32] Fix failing flow tests Summary: Metro Bundler ships with `react_native_fb` flow annotations whereas our repo is set to ignore `react_native_oss` flow issues. For now, a hot fix is to make our repo ignore both. Detailed discussion happens here: https://github.com/facebook/metro-bundler/commit/87cfc05ea67e19c367e24e644915b49024ce3f2a#commitcomment-24884835 Those changes have been applied to `0.50.0-rc.0` in order to make it green. Closes https://github.com/facebook/react-native/pull/16310 Differential Revision: D6031378 Pulled By: hramos fbshipit-source-id: 491d866bb35bd6c30a27dbc04586b15144a8efb2 --- .flowconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.flowconfig b/.flowconfig index 6ceb63fe5af3e7..8556171249fdaa 100644 --- a/.flowconfig +++ b/.flowconfig @@ -46,8 +46,8 @@ suppress_type=$FlowFixMeProps suppress_type=$FlowFixMeState suppress_type=$FixMe -suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native_oss[a-z,_]*\\)?)\\) -suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native_oss[a-z,_]*\\)?)\\)?:? #[0-9]+ +suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*[react_native_oss|react_native_fb][a-z,_]*\\)?)\\) +suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*[react_native_oss|react_native_fb][a-z,_]*\\)?)\\)?:? #[0-9]+ suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError From f9047cdfafe2f7988c90cd5d2fc31c41382082ed Mon Sep 17 00:00:00 2001 From: Scott Yost Date: Thu, 12 Oct 2017 09:00:02 -0700 Subject: [PATCH 10/32] Revert D5932093: [RAMBO] Final fixes Differential Revision: D5932093 fbshipit-source-id: 8e6ac0482a4c5a9ae4549be73d316df919613c9b --- ReactCommon/cxxreact/JSCExecutor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 3dfe2db47257d4..486961001ab1a0 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -651,7 +651,7 @@ namespace facebook { void JSCExecutor::loadModule(uint32_t bundleId, uint32_t moduleId) { auto module = m_bundleRegistry->getModule(bundleId, moduleId); auto sourceUrl = String::createExpectingAscii(m_context, module.name); - auto source = adoptString(std::unique_ptr(new JSBigStdString(module.code))); + auto source = String::createExpectingAscii(m_context, module.code); evaluateScript(m_context, source, sourceUrl); } From 7997a2b74950f0d76d5b5942aa2450f2b9207bf0 Mon Sep 17 00:00:00 2001 From: Michael Lee Date: Thu, 12 Oct 2017 09:07:05 -0700 Subject: [PATCH 11/32] Fix platform flags to also check for linux Reviewed By: danzimm Differential Revision: D6036995 fbshipit-source-id: 98d7f19eb80b72090b26252864a6bd41a3295462 --- ReactAndroid/src/main/jni/third-party/glibc/BUCK | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ReactAndroid/src/main/jni/third-party/glibc/BUCK b/ReactAndroid/src/main/jni/third-party/glibc/BUCK index c1744771afa3c7..4355e013c14760 100644 --- a/ReactAndroid/src/main/jni/third-party/glibc/BUCK +++ b/ReactAndroid/src/main/jni/third-party/glibc/BUCK @@ -6,7 +6,7 @@ include_defs("//ReactAndroid/DEFS") # libpthread is implicitly included in the android runtime so, when building # on an android platform, we don't do anything. -deprecated_prebuilt_cxx_library( +prebuilt_cxx_library( name = "pthread", exported_platform_linker_flags = [ ( @@ -14,7 +14,7 @@ deprecated_prebuilt_cxx_library( [], ), ( - "default", + "^(default|linux)", ["-lpthread"], ), ( @@ -28,7 +28,7 @@ deprecated_prebuilt_cxx_library( ], ) -deprecated_prebuilt_cxx_library( +prebuilt_cxx_library( name = "dl", exported_linker_flags = [ "-ldl", @@ -39,7 +39,7 @@ deprecated_prebuilt_cxx_library( ], ) -deprecated_prebuilt_cxx_library( +prebuilt_cxx_library( name = "m", exported_linker_flags = [ "-lm", @@ -50,7 +50,7 @@ deprecated_prebuilt_cxx_library( ], ) -deprecated_prebuilt_cxx_library( +prebuilt_cxx_library( name = "rt", exported_platform_linker_flags = [ ( @@ -58,7 +58,7 @@ deprecated_prebuilt_cxx_library( [], ), ( - "default", + "^(default|linux)", ["-lrt"], ), ], From 452ac1b58e5eb5db470fbda9cfdbded277e92ccc Mon Sep 17 00:00:00 2001 From: Alex Dvornikov Date: Thu, 12 Oct 2017 09:20:17 -0700 Subject: [PATCH 12/32] Added "fetchBundle" global function Reviewed By: jeanlauliac Differential Revision: D5985425 fbshipit-source-id: 72de85d354e85b8f7d98c95d5aa5348484d26204 --- Libraries/Core/InitializeCore.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Libraries/Core/InitializeCore.js b/Libraries/Core/InitializeCore.js index 4cd6754ee01f6d..4531b114efd90f 100644 --- a/Libraries/Core/InitializeCore.js +++ b/Libraries/Core/InitializeCore.js @@ -222,6 +222,26 @@ BatchedBridge.registerLazyCallableModule('RCTDeviceEventEmitter', () => require( BatchedBridge.registerLazyCallableModule('RCTNativeAppEventEmitter', () => require('RCTNativeAppEventEmitter')); BatchedBridge.registerLazyCallableModule('PerformanceLogger', () => require('PerformanceLogger')); +global.fetchBundle = function( + bundleId: number, + callback: (?Error) => void, +) { + const {BundleFetcher} = require('NativeModules'); + if (!BundleFetcher) { + throw new Error('BundleFetcher is missing'); + } + + BundleFetcher.fetchBundle(bundleId, (errorObject: ?{message: string, code: string}) => { + if (errorObject) { + const error = new Error(errorObject.message); + (error: any).code = errorObject.code; + callback(error); + } + + callback(null); + }); +}; + // Set up devtools if (__DEV__) { if (!global.__RCTProfileIsProfiling) { From f2c6877b91963878f36ec42f5f865427bc69488c Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Thu, 12 Oct 2017 09:41:26 -0700 Subject: [PATCH 13/32] Fixed crash on attempt to update local data of deallocated shadow node Summary: Trivial. That's okay that sometimes shadowNodes and views hierarchies have lack of synchonization. Reviewed By: sahrens Differential Revision: D6040022 fbshipit-source-id: 6b49a82317b620b66a87441719fddcafb1f27934 --- .../java/com/facebook/react/uimanager/UIImplementation.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java index 608187ff6d6017..f009512772a024 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java @@ -243,8 +243,10 @@ public void setViewLocalData(int tag, Object data) { ReactShadowNode shadowNode = mShadowNodeRegistry.getNode(tag); if (shadowNode == null) { - throw new IllegalViewOperationException( - "Trying to set local data for view with unknown tag: " + tag); + FLog.w( + ReactConstants.TAG, + "Attempt to set local data for view with unknown tag: " + tag); + return; } shadowNode.setLocalData(data); From 224d29447f3a90c9c20b48c990d4f885c4f97a3d Mon Sep 17 00:00:00 2001 From: alejandro garcia Date: Thu, 12 Oct 2017 11:32:30 -0700 Subject: [PATCH 14/32] Fixed navigation template Summary: - [x] Explain the **motivation** for making this change. It fixes #14313 Closes https://github.com/facebook/react-native/pull/14495 Differential Revision: D6042094 Pulled By: hramos fbshipit-source-id: d70e42bfee0a22882bad91cb885fb0cfc91c7d38 --- local-cli/templates/HelloNavigation/App.js | 2 - local-cli/templates/HelloNavigation/README.md | 8 ++-- .../HelloNavigation/dependencies.json | 2 +- .../views/HomeScreenTabNavigator.js | 2 - .../views/chat/ChatListScreen.js | 45 ++++++++----------- .../HelloNavigation/views/chat/ChatScreen.js | 34 ++++++-------- .../views/welcome/WelcomeScreen.js | 23 ++++------ .../views/welcome/WelcomeText.android.js | 2 - .../views/welcome/WelcomeText.ios.js | 2 - 9 files changed, 47 insertions(+), 73 deletions(-) diff --git a/local-cli/templates/HelloNavigation/App.js b/local-cli/templates/HelloNavigation/App.js index 9bd3691aa4c4e6..8b422a767b8bc7 100644 --- a/local-cli/templates/HelloNavigation/App.js +++ b/local-cli/templates/HelloNavigation/App.js @@ -1,5 +1,3 @@ -'use strict'; - /** * This is an example React Native app demonstrates ListViews, text input and * navigation between a few screens. diff --git a/local-cli/templates/HelloNavigation/README.md b/local-cli/templates/HelloNavigation/README.md index 110a605046931f..2c3d94b4c9dd67 100644 --- a/local-cli/templates/HelloNavigation/README.md +++ b/local-cli/templates/HelloNavigation/README.md @@ -1,6 +1,6 @@ # App template for new React Native apps -This is a simple React Native app template which demonstrates a few basics concepts such as navigation between a few screens, ListViews, and handling text input. +This is a simple React Native app template which demonstrates a few basics concepts such as navigation between a few screens, FlatLists, and handling text input. Android Example @@ -13,7 +13,7 @@ The idea is to make it easier for people to get started with React Native. Curre - Navigating between screens - Handling text input and the software keyboard -This app serves as a template used by `react-native init` so it is easier for anyone to get up and running quickly by having an app with a few screens and a ListView ready to go. +This app serves as a template used by `react-native init` so it is easier for anyone to get up and running quickly by having an app with a few screens and a FlatList ready to go. ### Best practices @@ -21,7 +21,7 @@ Another purpose of this app is to define best practices such as the folder struc ## Not using Redux -This template intentionally doesn't use Redux. After discussing with a few people who have experience using Redux we concluded that adding Redux to this app targeted at beginners would make the code more confusing, and wouldn't clearly show the benefits of Redux (because the app is too small). There are already a few concepts to grasp - the React component lifecycle, rendeing lists, using async / await, handling the software keyboard. We thought that's the maximum amount of things to learn at once. It's better for everyone to see patterns in their codebase as the app grows and decide for themselves whether and when they need Redux. See also the post [You Might Not Need Redux](https://medium.com/@dan_abramov/you-might-not-need-redux-be46360cf367#.f3q7kq4b3) by [Dan Abramov](https://twitter.com/dan_abramov). +This template intentionally doesn't use Redux. After discussing with a few people who have experience using Redux we concluded that adding Redux to this app targeted at beginners would make the code more confusing, and wouldn't clearly show the benefits of Redux (because the app is too small). There are already a few concepts to grasp - the React component lifecycle, rendering lists, using async / await, handling the software keyboard. We thought that's the maximum amount of things to learn at once. It's better for everyone to see patterns in their codebase as the app grows and decide for themselves whether and when they need Redux. See also the post [You Might Not Need Redux](https://medium.com/@dan_abramov/you-might-not-need-redux-be46360cf367#.f3q7kq4b3) by [Dan Abramov](https://twitter.com/dan_abramov). ## Not using Flow (for now) @@ -34,7 +34,7 @@ We need your feedback. Do you have a lot of experience building React Native app ## How to use the template ``` -$ react-native init MyApp --version 0.42.0-rc.2 --template navigation +$ react-native init MyApp --template navigation $ cd MyApp $ react-native run-android $ react-native run-ios diff --git a/local-cli/templates/HelloNavigation/dependencies.json b/local-cli/templates/HelloNavigation/dependencies.json index b6a0896fc7c8ee..2317f38b79c837 100644 --- a/local-cli/templates/HelloNavigation/dependencies.json +++ b/local-cli/templates/HelloNavigation/dependencies.json @@ -1,3 +1,3 @@ { - "react-navigation": "1.0.0-beta.5" + "react-navigation": "1.0.0-beta.11" } diff --git a/local-cli/templates/HelloNavigation/views/HomeScreenTabNavigator.js b/local-cli/templates/HelloNavigation/views/HomeScreenTabNavigator.js index 7ccfba465562ef..36ca5af70fe227 100644 --- a/local-cli/templates/HelloNavigation/views/HomeScreenTabNavigator.js +++ b/local-cli/templates/HelloNavigation/views/HomeScreenTabNavigator.js @@ -1,5 +1,3 @@ -'use strict'; - import { TabNavigator } from 'react-navigation'; import ChatListScreen from './chat/ChatListScreen'; diff --git a/local-cli/templates/HelloNavigation/views/chat/ChatListScreen.js b/local-cli/templates/HelloNavigation/views/chat/ChatListScreen.js index c899f5dec74d41..4d4fcc093e9ca1 100644 --- a/local-cli/templates/HelloNavigation/views/chat/ChatListScreen.js +++ b/local-cli/templates/HelloNavigation/views/chat/ChatListScreen.js @@ -1,10 +1,8 @@ -'use strict'; - import React, { Component } from 'react'; import { ActivityIndicator, Image, - ListView, + FlatList, Platform, StyleSheet, View, @@ -16,47 +14,41 @@ export default class ChatListScreen extends Component { static navigationOptions = { title: 'Chats', - header: { - visible: Platform.OS === 'ios', - }, - tabBar: { - icon: ({ tintColor }) => ( - - ), - }, + header: Platform.OS === 'ios' ? undefined : null, + tabBarIcon: ({ tintColor }) => ( + + ), } constructor(props) { super(props); - const ds = new ListView.DataSource({rowHasChanged: (r1, r2) => r1 !== r2}); this.state = { isLoading: true, - dataSource: ds, }; } async componentDidMount() { const chatList = await Backend.fetchChatList(); this.setState((prevState) => ({ - dataSource: prevState.dataSource.cloneWithRows(chatList), + chatList, isLoading: false, })); } - // Binding the function so it can be passed to ListView below - // and 'this' works properly inside renderRow - renderRow = (name) => { + // Binding the function so it can be passed to FlatList below + // and 'this' works properly inside renderItem + renderItem = ({ item }) => { return ( { // Start fetching in parallel with animating this.props.navigation.navigate('Chat', { - name: name, + name: item, }); }} /> @@ -72,9 +64,10 @@ export default class ChatListScreen extends Component { ); } return ( - index} style={styles.listView} /> ); diff --git a/local-cli/templates/HelloNavigation/views/chat/ChatScreen.js b/local-cli/templates/HelloNavigation/views/chat/ChatScreen.js index 7aed21db8a2163..6f1dfff321315f 100644 --- a/local-cli/templates/HelloNavigation/views/chat/ChatScreen.js +++ b/local-cli/templates/HelloNavigation/views/chat/ChatScreen.js @@ -1,10 +1,8 @@ -'use strict'; - import React, { Component } from 'react'; import { ActivityIndicator, Button, - ListView, + FlatList, StyleSheet, Text, TextInput, @@ -15,16 +13,13 @@ import Backend from '../../lib/Backend'; export default class ChatScreen extends Component { - static navigationOptions = { - title: (navigation) => `Chat with ${navigation.state.params.name}`, - } - + static navigationOptions = ({ navigation }) => ({ + title: `Chat with ${navigation.state.params.name}`, + }); constructor(props) { super(props); - const ds = new ListView.DataSource({rowHasChanged: (r1, r2) => r1 !== r2}); this.state = { messages: [], - dataSource: ds, myMessage: '', isLoading: true, }; @@ -47,7 +42,6 @@ export default class ChatScreen extends Component { } this.setState((prevState) => ({ messages: chat.messages, - dataSource: prevState.dataSource.cloneWithRows(chat.messages), isLoading: false, })); } @@ -82,7 +76,6 @@ export default class ChatScreen extends Component { ]; return { messages: messages, - dataSource: prevState.dataSource.cloneWithRows(messages), myMessage: '', }; }); @@ -93,10 +86,10 @@ export default class ChatScreen extends Component { this.setState({myMessage: event.nativeEvent.text}); } - renderRow = (message) => ( + renderItem = ({ item }) => ( - {message.name} - {message.text} + {item.name} + {item.text} ) @@ -110,12 +103,13 @@ export default class ChatScreen extends Component { } return ( - + index} + style={styles.listView} + /> + { this.textInput = textInput; }} diff --git a/local-cli/templates/HelloNavigation/views/welcome/WelcomeScreen.js b/local-cli/templates/HelloNavigation/views/welcome/WelcomeScreen.js index 4b993634d01739..62b7fba55c5b06 100644 --- a/local-cli/templates/HelloNavigation/views/welcome/WelcomeScreen.js +++ b/local-cli/templates/HelloNavigation/views/welcome/WelcomeScreen.js @@ -1,5 +1,3 @@ -'use strict'; - import React, { Component } from 'react'; import { Image, @@ -14,18 +12,15 @@ export default class WelcomeScreen extends Component { static navigationOptions = { title: 'Welcome', - header: { - visible: Platform.OS === 'ios', - }, - tabBar: { - icon: ({ tintColor }) => ( - - ), - }, + // You can now set header: null on any component to hide the header + header: Platform.OS === 'ios' ? undefined : null, + tabBarIcon: ({ tintColor }) => ( + + ), } render() { diff --git a/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.android.js b/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.android.js index 7a12d97f8f91b0..bf77b5101c7a3a 100644 --- a/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.android.js +++ b/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.android.js @@ -1,5 +1,3 @@ -'use strict'; - import React, { Component } from 'react'; import { StyleSheet, diff --git a/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.ios.js b/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.ios.js index bf90253014f7e4..8e040208abbf39 100644 --- a/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.ios.js +++ b/local-cli/templates/HelloNavigation/views/welcome/WelcomeText.ios.js @@ -1,5 +1,3 @@ -'use strict'; - import React, { Component } from 'react'; import { StyleSheet, From 4192790f05269181a1f50f31ce94e20c9a8ea2bf Mon Sep 17 00:00:00 2001 From: Sergei Dryganets Date: Thu, 12 Oct 2017 11:49:42 -0700 Subject: [PATCH 15/32] more detailed CxxModule logging Summary: Cxx module code swallows c++ exception details with sarcastic comment let native developer figure it out. Now instead of swallowing it, we print as much information as we can for different exception types. Still not ideal but way more informative. Have a crash in your c++ module and try to figure it out without this change. Closes https://github.com/facebook/react-native/pull/16193 Differential Revision: D6040038 Pulled By: javache fbshipit-source-id: 3fbe838383ca13395e21f74c9549474f6329cfeb --- ReactCommon/cxxreact/CxxNativeModule.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ReactCommon/cxxreact/CxxNativeModule.cpp b/ReactCommon/cxxreact/CxxNativeModule.cpp index f050ae1d2b341c..a8e511fbec4aa8 100644 --- a/ReactCommon/cxxreact/CxxNativeModule.cpp +++ b/ReactCommon/cxxreact/CxxNativeModule.cpp @@ -4,7 +4,7 @@ #include "Instance.h" #include - +#include #include #include "JsArgumentHelpers.h" @@ -12,7 +12,6 @@ #include "MessageQueueThread.h" using facebook::xplat::module::CxxModule; - namespace facebook { namespace react { @@ -140,9 +139,14 @@ void CxxNativeModule::invoke(unsigned int reactMethodId, folly::dynamic&& params method.func(std::move(params), first, second); } catch (const facebook::xplat::JsArgumentException& ex) { throw; + } catch (std::exception& e) { + LOG(ERROR) << "std::exception. Method call " << method.name.c_str() << " failed: " << e.what(); + std::terminate(); + } catch (std::string& error) { + LOG(ERROR) << "std::string. Method call " << method.name.c_str() << " failed: " << error.c_str(); + std::terminate(); } catch (...) { - // This means some C++ code is buggy. As above, we fail hard so the C++ - // developer can debug and fix it. + LOG(ERROR) << "Method call " << method.name.c_str() << " failed. unknown error"; std::terminate(); } }); From dd400f842b416b421d73ce3d308b73e26a5d3577 Mon Sep 17 00:00:00 2001 From: Alex Dvornikov Date: Thu, 12 Oct 2017 12:30:24 -0700 Subject: [PATCH 16/32] add "jsBundlesDirectory" method to RCTBridgeDelegate Differential Revision: D6030185 fbshipit-source-id: 58d6f9d0d412c7ad0f83af9ae4df01c4dc1178bc --- React/Base/RCTBridgeDelegate.h | 7 +++++++ React/CxxBridge/RCTCxxBridge.mm | 7 ++++++- ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.cpp | 4 ++-- ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.h | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/React/Base/RCTBridgeDelegate.h b/React/Base/RCTBridgeDelegate.h index 2327b1c1de9522..2ec89fd57efeda 100644 --- a/React/Base/RCTBridgeDelegate.h +++ b/React/Base/RCTBridgeDelegate.h @@ -121,4 +121,11 @@ - (void)loadSourceForBridge:(RCTBridge *)bridge withBlock:(RCTSourceLoadBlock)loadCallback; +/** + * Specifies the path to folder where additional bundles are located + * + * @experimental + */ +- (NSURL *)jsBundlesDirectory; + @end diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index a499a8968added..f95b19b57ae8cb 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -1190,7 +1190,12 @@ - (void)executeApplicationScript:(NSData *)script [self->_performanceLogger markStopForTag:RCTPLRAMBundleLoad]; [self->_performanceLogger setValue:scriptStr->size() forTag:RCTPLRAMStartupCodeSize]; if (self->_reactInstance) { - auto registry = std::make_unique(std::move(ramBundle), sourceUrlStr.UTF8String); + NSString *jsBundlesDirectory = [self.delegate respondsToSelector:@selector(jsBundlesDirectory)] + ? [[self.delegate jsBundlesDirectory].path stringByAppendingString:@"/"] + : nil; + auto registry = jsBundlesDirectory != nil + ? std::make_unique(std::move(ramBundle), jsBundlesDirectory.UTF8String) + : std::make_unique(std::move(ramBundle)); self->_reactInstance->loadRAMBundle(std::move(registry), std::move(scriptStr), sourceUrlStr.UTF8String, !async); } diff --git a/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.cpp b/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.cpp index 2e678802a58837..44a58e36181137 100644 --- a/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.cpp +++ b/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.cpp @@ -10,8 +10,8 @@ namespace facebook { namespace react { -JSIndexedRAMBundleRegistry::JSIndexedRAMBundleRegistry(std::unique_ptr mainBundle, const std::string& entryFile): - RAMBundleRegistry(std::move(mainBundle)), m_baseDirectoryPath(jsBundlesDir(entryFile)) {} +JSIndexedRAMBundleRegistry::JSIndexedRAMBundleRegistry(std::unique_ptr mainBundle, const std::string& baseDirectoryPath): +RAMBundleRegistry(std::move(mainBundle)), m_baseDirectoryPath(baseDirectoryPath) {} std::unique_ptr JSIndexedRAMBundleRegistry::bundleById(uint32_t index) const { std::string bundlePathById = m_baseDirectoryPath + toString(index) + ".jsbundle"; diff --git a/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.h b/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.h index 8dd213a21621fa..199f26772a54ce 100644 --- a/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.h +++ b/ReactCommon/cxxreact/JSIndexedRAMBundleRegistry.h @@ -13,7 +13,7 @@ namespace react { class RN_EXPORT JSIndexedRAMBundleRegistry: public RAMBundleRegistry { public: - JSIndexedRAMBundleRegistry(std::unique_ptr mainBundle, const std::string& entryFile); + JSIndexedRAMBundleRegistry(std::unique_ptr mainBundle, const std::string& baseDirectoryPath); protected: virtual std::unique_ptr bundleById(uint32_t index) const override; From 2b4ff6ea19dc674cf035ee419daa132cde8d1f5e Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Thu, 12 Oct 2017 13:02:33 -0700 Subject: [PATCH 17/32] Native Animated - Restore default values when removing props on Android Summary: Rebased version of #12842 that was reverted because of failing fb internal tests. Closes https://github.com/facebook/react-native/pull/15919 Differential Revision: D5823956 Pulled By: hramos fbshipit-source-id: 4ece19a403f5ebbe4829c4c26696ea0575ab1d0e --- .../react/animated/NativeAnimatedModule.java | 98 +++++++++++-------- .../animated/NativeAnimatedNodesManager.java | 31 +++--- .../react/animated/PropsAnimatedNode.java | 68 +++++++++---- .../react/uimanager/UIImplementation.java | 4 + .../react/uimanager/UIManagerModule.java | 26 ++++- .../uimanager/UIManagerModuleListener.java | 21 ++++ .../react/uimanager/UIViewOperationQueue.java | 4 + .../java/com/facebook/react/animated/BUCK | 1 + .../NativeAnimatedNodeTraversalTest.java | 47 +++++++++ 9 files changed, 229 insertions(+), 71 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java index f03cc8a460fbd5..0591f5dbb7e14c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java @@ -17,17 +17,20 @@ import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.LifecycleEventListener; -import com.facebook.react.bridge.OnBatchCompleteListener; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.WritableMap; +import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.modules.core.DeviceEventManagerModule; import com.facebook.react.modules.core.ReactChoreographer; import com.facebook.react.uimanager.GuardedFrameCallback; +import com.facebook.react.uimanager.NativeViewHierarchyManager; +import com.facebook.react.uimanager.UIBlock; import com.facebook.react.uimanager.UIManagerModule; +import com.facebook.react.uimanager.UIManagerModuleListener; /** * Module that exposes interface for creating and managing animated nodes on the "native" side. @@ -72,7 +75,7 @@ */ @ReactModule(name = NativeAnimatedModule.NAME) public class NativeAnimatedModule extends ReactContextBaseJavaModule implements - OnBatchCompleteListener, LifecycleEventListener { + LifecycleEventListener, UIManagerModuleListener { protected static final String NAME = "NativeAnimatedModule"; @@ -80,11 +83,10 @@ private interface UIThreadOperation { void execute(NativeAnimatedNodesManager animatedNodesManager); } - private final Object mOperationsCopyLock = new Object(); private final GuardedFrameCallback mAnimatedFrameCallback; private final ReactChoreographer mReactChoreographer; private ArrayList mOperations = new ArrayList<>(); - private volatile @Nullable ArrayList mReadyOperations = null; + private ArrayList mPreOperations = new ArrayList<>(); private @Nullable NativeAnimatedNodesManager mNodesManager; @@ -95,26 +97,9 @@ public NativeAnimatedModule(ReactApplicationContext reactContext) { mAnimatedFrameCallback = new GuardedFrameCallback(reactContext) { @Override protected void doFrameGuarded(final long frameTimeNanos) { - if (mNodesManager == null) { - UIManagerModule uiManager = getReactApplicationContext() - .getNativeModule(UIManagerModule.class); - mNodesManager = new NativeAnimatedNodesManager(uiManager); - } - - ArrayList operations; - synchronized (mOperationsCopyLock) { - operations = mReadyOperations; - mReadyOperations = null; - } - - if (operations != null) { - for (int i = 0, size = operations.size(); i < size; i++) { - operations.get(i).execute(mNodesManager); - } - } - - if (mNodesManager.hasActiveAnimations()) { - mNodesManager.runUpdates(frameTimeNanos); + NativeAnimatedNodesManager nodesManager = getNodesManager(); + if (nodesManager.hasActiveAnimations()) { + nodesManager.runUpdates(frameTimeNanos); } // TODO: Would be great to avoid adding this callback in case there are no active animations @@ -130,7 +115,10 @@ protected void doFrameGuarded(final long frameTimeNanos) { @Override public void initialize() { - getReactApplicationContext().addLifecycleEventListener(this); + ReactApplicationContext reactCtx = getReactApplicationContext(); + UIManagerModule uiManager = reactCtx.getNativeModule(UIManagerModule.class); + reactCtx.addLifecycleEventListener(this); + uiManager.addUIManagerListener(this); } @Override @@ -139,24 +127,32 @@ public void onHostResume() { } @Override - public void onBatchComplete() { - // Note: The order of executing onBatchComplete handler (especially in terms of onBatchComplete - // from the UIManagerModule) doesn't matter as we only enqueue operations for the UI thread to - // be executed from here. Thanks to ReactChoreographer all the operations from here are going - // to be executed *after* all the operations enqueued by UIManager as the callback type that we - // use for ReactChoreographer (CallbackType.NATIVE_ANIMATED_MODULE) is run after callbacks that - // UIManager uses. - ArrayList operations = mOperations.isEmpty() ? null : mOperations; - if (operations != null) { - mOperations = new ArrayList<>(); - synchronized (mOperationsCopyLock) { - if (mReadyOperations == null) { - mReadyOperations = operations; - } else { - mReadyOperations.addAll(operations); + public void willDispatchViewUpdates(final UIManagerModule uiManager) { + if (mOperations.isEmpty() && mPreOperations.isEmpty()) { + return; + } + final ArrayList preOperations = mPreOperations; + final ArrayList operations = mOperations; + mPreOperations = new ArrayList<>(); + mOperations = new ArrayList<>(); + uiManager.prependUIBlock(new UIBlock() { + @Override + public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { + NativeAnimatedNodesManager nodesManager = getNodesManager(); + for (UIThreadOperation operation : preOperations) { + operation.execute(nodesManager); } } - } + }); + uiManager.addUIBlock(new UIBlock() { + @Override + public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { + NativeAnimatedNodesManager nodesManager = getNodesManager(); + for (UIThreadOperation operation : operations) { + operation.execute(nodesManager); + } + } + }); } @Override @@ -174,6 +170,15 @@ public String getName() { return NAME; } + private NativeAnimatedNodesManager getNodesManager() { + if (mNodesManager == null) { + UIManagerModule uiManager = getReactApplicationContext().getNativeModule(UIManagerModule.class); + mNodesManager = new NativeAnimatedNodesManager(uiManager); + } + + return mNodesManager; + } + private void clearFrameCallback() { Assertions.assertNotNull(mReactChoreographer).removeFrameCallback( ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE, @@ -186,6 +191,11 @@ private void enqueueFrameCallback() { mAnimatedFrameCallback); } + @VisibleForTesting + public void setNodesManager(NativeAnimatedNodesManager nodesManager) { + mNodesManager = nodesManager; + } + @ReactMethod public void createAnimatedNode(final int tag, final ReadableMap config) { mOperations.add(new UIThreadOperation() { @@ -336,6 +346,12 @@ public void execute(NativeAnimatedNodesManager animatedNodesManager) { @ReactMethod public void disconnectAnimatedNodeFromView(final int animatedNodeTag, final int viewTag) { + mPreOperations.add(new UIThreadOperation() { + @Override + public void execute(NativeAnimatedNodesManager animatedNodesManager) { + animatedNodesManager.restoreDefaultValues(animatedNodeTag, viewTag); + } + }); mOperations.add(new UIThreadOperation() { @Override public void execute(NativeAnimatedNodesManager animatedNodesManager) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java index 68b0ad62a3e808..d65b20fc503794 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java @@ -90,7 +90,7 @@ public void createAnimatedNode(int tag, ReadableMap config) { } else if ("value".equals(type)) { node = new ValueAnimatedNode(config); } else if ("props".equals(type)) { - node = new PropsAnimatedNode(config, this); + node = new PropsAnimatedNode(config, this, mUIImplementation); } else if ("interpolation".equals(type)) { node = new InterpolationAnimatedNode(config); } else if ("addition".equals(type)) { @@ -287,11 +287,7 @@ public void connectAnimatedNodeToView(int animatedNodeTag, int viewTag) { "of type " + PropsAnimatedNode.class.getName()); } PropsAnimatedNode propsAnimatedNode = (PropsAnimatedNode) node; - if (propsAnimatedNode.mConnectedViewTag != -1) { - throw new JSApplicationIllegalArgumentException("Animated node " + animatedNodeTag + " is " + - "already attached to a view"); - } - propsAnimatedNode.mConnectedViewTag = viewTag; + propsAnimatedNode.connectToView(viewTag); mUpdatedNodes.put(animatedNodeTag, node); } @@ -306,11 +302,24 @@ public void disconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag) { "of type " + PropsAnimatedNode.class.getName()); } PropsAnimatedNode propsAnimatedNode = (PropsAnimatedNode) node; - if (propsAnimatedNode.mConnectedViewTag != viewTag) { - throw new JSApplicationIllegalArgumentException("Attempting to disconnect view that has " + - "not been connected with the given animated node"); + propsAnimatedNode.disconnectFromView(viewTag); + } + + public void restoreDefaultValues(int animatedNodeTag, int viewTag) { + AnimatedNode node = mAnimatedNodes.get(animatedNodeTag); + // Restoring default values needs to happen before UIManager operations so it is + // possible the node hasn't been created yet if it is being connected and + // disconnected in the same batch. In that case we don't need to restore + // default values since it will never actually update the view. + if (node == null) { + return; } - propsAnimatedNode.mConnectedViewTag = -1; + if (!(node instanceof PropsAnimatedNode)) { + throw new JSApplicationIllegalArgumentException("Animated node connected to view should be" + + "of type " + PropsAnimatedNode.class.getName()); + } + PropsAnimatedNode propsAnimatedNode = (PropsAnimatedNode) node; + propsAnimatedNode.restoreDefaultValues(); } public void addAnimatedEventToView(int viewTag, String eventName, ReadableMap eventMapping) { @@ -516,7 +525,7 @@ private void updateNodes(List nodes) { if (nextNode instanceof PropsAnimatedNode) { // Send property updates to native view manager try { - ((PropsAnimatedNode) nextNode).updateView(mUIImplementation); + ((PropsAnimatedNode) nextNode).updateView(); } catch (IllegalViewOperationException e) { // An exception is thrown if the view hasn't been created yet. This can happen because views are // created in batches. If this particular view didn't make it into a batch yet, the view won't diff --git a/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java b/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java index de5c3e6f0278e9..036f88435cc21e 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/animated/PropsAnimatedNode.java @@ -9,6 +9,7 @@ package com.facebook.react.animated; +import com.facebook.react.bridge.JSApplicationIllegalArgumentException; import com.facebook.react.bridge.JavaOnlyMap; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.ReadableMapKeySetIterator; @@ -27,47 +28,78 @@ */ /*package*/ class PropsAnimatedNode extends AnimatedNode { - /*package*/ int mConnectedViewTag = -1; - + private int mConnectedViewTag = -1; private final NativeAnimatedNodesManager mNativeAnimatedNodesManager; - private final Map mPropMapping; + private final UIImplementation mUIImplementation; + private final Map mPropNodeMapping; + // This is the backing map for `mDiffMap` we can mutate this to update it instead of having to + // create a new one for each update. + private final JavaOnlyMap mPropMap; + private final ReactStylesDiffMap mDiffMap; - PropsAnimatedNode(ReadableMap config, NativeAnimatedNodesManager nativeAnimatedNodesManager) { + PropsAnimatedNode(ReadableMap config, NativeAnimatedNodesManager nativeAnimatedNodesManager, UIImplementation uiImplementation) { ReadableMap props = config.getMap("props"); ReadableMapKeySetIterator iter = props.keySetIterator(); - mPropMapping = new HashMap<>(); + mPropNodeMapping = new HashMap<>(); while (iter.hasNextKey()) { String propKey = iter.nextKey(); int nodeIndex = props.getInt(propKey); - mPropMapping.put(propKey, nodeIndex); + mPropNodeMapping.put(propKey, nodeIndex); } + mPropMap = new JavaOnlyMap(); + mDiffMap = new ReactStylesDiffMap(mPropMap); mNativeAnimatedNodesManager = nativeAnimatedNodesManager; + mUIImplementation = uiImplementation; + } + + public void connectToView(int viewTag) { + if (mConnectedViewTag != -1) { + throw new JSApplicationIllegalArgumentException("Animated node " + mTag + " is " + + "already attached to a view"); + } + mConnectedViewTag = viewTag; + } + + public void disconnectFromView(int viewTag) { + if (mConnectedViewTag != viewTag) { + throw new JSApplicationIllegalArgumentException("Attempting to disconnect view that has " + + "not been connected with the given animated node"); + } + + mConnectedViewTag = -1; } - public final void updateView(UIImplementation uiImplementation) { + public void restoreDefaultValues() { + ReadableMapKeySetIterator it = mPropMap.keySetIterator(); + while(it.hasNextKey()) { + mPropMap.putNull(it.nextKey()); + } + + mUIImplementation.synchronouslyUpdateViewOnUIThread( + mConnectedViewTag, + mDiffMap); + } + + public final void updateView() { if (mConnectedViewTag == -1) { - throw new IllegalStateException("Node has not been attached to a view"); + return; } - JavaOnlyMap propsMap = new JavaOnlyMap(); - for (Map.Entry entry : mPropMapping.entrySet()) { + for (Map.Entry entry : mPropNodeMapping.entrySet()) { @Nullable AnimatedNode node = mNativeAnimatedNodesManager.getNodeById(entry.getValue()); if (node == null) { throw new IllegalArgumentException("Mapped property node does not exists"); } else if (node instanceof StyleAnimatedNode) { - ((StyleAnimatedNode) node).collectViewUpdates(propsMap); + ((StyleAnimatedNode) node).collectViewUpdates(mPropMap); } else if (node instanceof ValueAnimatedNode) { - propsMap.putDouble(entry.getKey(), ((ValueAnimatedNode) node).getValue()); + mPropMap.putDouble(entry.getKey(), ((ValueAnimatedNode) node).getValue()); } else { throw new IllegalArgumentException("Unsupported type of node used in property node " + node.getClass()); } } - // TODO: Reuse propsMap and stylesDiffMap objects - note that in subsequent animation steps - // for a given node most of the time we will be creating the same set of props (just with - // different values). We can take advantage on that and optimize the way we allocate property - // maps (we also know that updating view props doesn't retain a reference to the styles object). - uiImplementation.synchronouslyUpdateViewOnUIThread( + + mUIImplementation.synchronouslyUpdateViewOnUIThread( mConnectedViewTag, - new ReactStylesDiffMap(propsMap)); + mDiffMap); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java index f009512772a024..160d43e4371e72 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java @@ -979,6 +979,10 @@ public void addUIBlock(UIBlock block) { mOperationsQueue.enqueueUIBlock(block); } + public void prependUIBlock(UIBlock block) { + mOperationsQueue.prependUIBlock(block); + } + public int resolveRootTagFromReactTag(int reactTag) { if (mShadowNodeRegistry.isRootNode(reactTag)) { return reactTag; diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java index 8988f597fc6c55..3018ccc0831b26 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModule.java @@ -39,6 +39,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.systrace.Systrace; import com.facebook.systrace.SystraceMessage; + +import java.util.ArrayList; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -112,6 +114,7 @@ public interface CustomEventNamesResolver { private final Map mCustomDirectEvents; private final UIImplementation mUIImplementation; private final MemoryTrimCallback mMemoryTrimCallback = new MemoryTrimCallback(); + private final List mListeners = new ArrayList<>(); private int mBatchId = 0; @@ -662,6 +665,9 @@ public void onBatchComplete() { SystraceMessage.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "onBatchCompleteUI") .arg("BatchId", batchId) .flush(); + for (UIManagerModuleListener listener : mListeners) { + listener.willDispatchViewUpdates(this); + } try { mUIImplementation.dispatchViewUpdates(batchId); } finally { @@ -699,10 +705,28 @@ public void execute (NativeViewHierarchyManager nvhm) { } }); */ - public void addUIBlock (UIBlock block) { + public void addUIBlock(UIBlock block) { mUIImplementation.addUIBlock(block); } + /** + * Schedule a block to be executed on the UI thread. Useful if you need to execute + * view logic before all currently queued view updates have completed. + * + * @param block that contains UI logic you want to execute. + */ + public void prependUIBlock(UIBlock block) { + mUIImplementation.prependUIBlock(block); + } + + public void addUIManagerListener(UIManagerModuleListener listener) { + mListeners.add(listener); + } + + public void removeUIManagerListener(UIManagerModuleListener listener) { + mListeners.remove(listener); + } + /** * Given a reactTag from a component, find its root node tag, if possible. * Otherwise, this will return 0. If the reactTag belongs to a root node, this diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java new file mode 100644 index 00000000000000..1dda859eb3f459 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleListener.java @@ -0,0 +1,21 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package com.facebook.react.uimanager; + +/** + * Listener used to hook into the UIManager update process. + */ +public interface UIManagerModuleListener { + /** + * Called right before view updates are dispatched at the end of a batch. This is useful if a + * module needs to add UIBlocks to the queue before it is flushed. + */ + void willDispatchViewUpdates(UIManagerModule uiManager); +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java index 62b231a2a679a1..00f6c42977c9e1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java @@ -758,6 +758,10 @@ public void enqueueUIBlock(UIBlock block) { mOperations.add(new UIBlockOperation(block)); } + public void prependUIBlock(UIBlock block) { + mOperations.add(0, new UIBlockOperation(block)); + } + /* package */ void dispatchViewUpdates( final int batchId, final long commitStartTime, final long layoutTime) { SystraceMessage.beginSection( diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK b/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK index a81a1ab62bf4ff..4a175cc5a52c00 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/BUCK @@ -19,6 +19,7 @@ rn_robolectric_test( react_native_target("java/com/facebook/react/animated:animated"), react_native_target("java/com/facebook/react/bridge:bridge"), react_native_target("java/com/facebook/react/common:common"), + react_native_target("java/com/facebook/react/modules/core:core"), react_native_target("java/com/facebook/react/uimanager:uimanager"), react_native_tests_target("java/com/facebook/react/bridge:testhelpers"), ], diff --git a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java index a1cb882b05f361..6f244c70be4baf 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java +++ b/ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java @@ -1059,4 +1059,51 @@ public Object answer(InvocationOnMock invocation) throws Throwable { verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); assertThat(stylesCaptor.getValue().getDouble("opacity", Double.NaN)).isEqualTo(10); } + + @Test + public void testRestoreDefaultProps() { + int viewTag = 1000; + int propsNodeTag = 3; + mNativeAnimatedNodesManager.createAnimatedNode( + 1, + JavaOnlyMap.of("type", "value", "value", 1d, "offset", 0d)); + mNativeAnimatedNodesManager.createAnimatedNode( + 2, + JavaOnlyMap.of("type", "style", "style", JavaOnlyMap.of("opacity", 1))); + mNativeAnimatedNodesManager.createAnimatedNode( + propsNodeTag, + JavaOnlyMap.of("type", "props", "props", JavaOnlyMap.of("style", 2))); + mNativeAnimatedNodesManager.connectAnimatedNodes(1, 2); + mNativeAnimatedNodesManager.connectAnimatedNodes(2, propsNodeTag); + mNativeAnimatedNodesManager.connectAnimatedNodeToView(propsNodeTag, viewTag); + + JavaOnlyArray frames = JavaOnlyArray.of(0d, 0.5d, 1d); + Callback animationCallback = mock(Callback.class); + mNativeAnimatedNodesManager.startAnimatingNode( + 1, + 1, + JavaOnlyMap.of("type", "frames", "frames", frames, "toValue", 0d), + animationCallback); + + ArgumentCaptor stylesCaptor = + ArgumentCaptor.forClass(ReactStylesDiffMap.class); + + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); + assertThat(stylesCaptor.getValue().getDouble("opacity", Double.NaN)).isEqualTo(1); + + for (int i = 0; i < frames.size(); i++) { + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.runUpdates(nextFrameTime()); + } + + verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); + assertThat(stylesCaptor.getValue().getDouble("opacity", Double.NaN)).isEqualTo(0); + + reset(mUIImplementationMock); + mNativeAnimatedNodesManager.restoreDefaultValues(propsNodeTag, viewTag); + verify(mUIImplementationMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture()); + assertThat(stylesCaptor.getValue().isNull("opacity")); + } } From fe62da34a6976bce894e130d0bebf49ea420d1a1 Mon Sep 17 00:00:00 2001 From: Max Sherman Date: Thu, 12 Oct 2017 15:27:03 -0700 Subject: [PATCH 18/32] Share logPerfMarker with hermes Reviewed By: mhorowitz Differential Revision: D5939688 fbshipit-source-id: ab6a86eb66a3d9501b9b5a55b562b277b5ab0620 --- .../main/jni/react/jni/AndroidJSCFactory.cpp | 50 +-------------- ReactAndroid/src/main/jni/react/jni/BUCK | 1 + .../src/main/jni/react/jni/ReactMarker.cpp | 62 +++++++++++++++++++ .../src/main/jni/react/jni/ReactMarker.h | 24 +++++++ 4 files changed, 89 insertions(+), 48 deletions(-) create mode 100644 ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp create mode 100644 ReactAndroid/src/main/jni/react/jni/ReactMarker.h diff --git a/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp b/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp index 52924b641362b4..0aba406fc24b86 100644 --- a/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp +++ b/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp @@ -13,6 +13,7 @@ #include "JSCPerfLogging.h" #include "JSLogging.h" +#include "ReactMarker.h" using namespace facebook::jni; @@ -21,53 +22,6 @@ namespace react { namespace { -class JReactMarker : public JavaClass { - public: - static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; - - static void logMarker(const std::string& marker) { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("logMarker"); - meth(cls, marker); - } - - static void logMarker(const std::string& marker, const std::string& tag) { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("logMarker"); - meth(cls, marker, tag); - } -}; - -void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) { - switch (markerId) { - case ReactMarker::RUN_JS_BUNDLE_START: - JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag); - break; - case ReactMarker::RUN_JS_BUNDLE_STOP: - JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag); - break; - case ReactMarker::CREATE_REACT_CONTEXT_STOP: - JReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); - break; - case ReactMarker::JS_BUNDLE_STRING_CONVERT_START: - JReactMarker::logMarker("loadApplicationScript_startStringConvert"); - break; - case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP: - JReactMarker::logMarker("loadApplicationScript_endStringConvert"); - break; - case ReactMarker::NATIVE_MODULE_SETUP_START: - JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag); - break; - case ReactMarker::NATIVE_MODULE_SETUP_STOP: - JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag); - break; - case ReactMarker::NATIVE_REQUIRE_START: - case ReactMarker::NATIVE_REQUIRE_STOP: - // These are not used on Android. - break; - } -} - ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) { auto jniEx = dynamic_cast(&ex); if (!jniEx) { @@ -126,7 +80,7 @@ namespace detail { void injectJSCExecutorAndroidPlatform() { // Inject some behavior into react/ - ReactMarker::logTaggedMarker = logPerfMarker; + JReactMarker::setLogPerfMarkerIfNeeded(); ExceptionHandling::platformErrorExtractor = extractJniError; JSCNativeHooks::loggingHook = nativeLoggingHook; JSCNativeHooks::nowHook = nativePerformanceNow; diff --git a/ReactAndroid/src/main/jni/react/jni/BUCK b/ReactAndroid/src/main/jni/react/jni/BUCK index 36f6faabae55df..32878cae633d41 100644 --- a/ReactAndroid/src/main/jni/react/jni/BUCK +++ b/ReactAndroid/src/main/jni/react/jni/BUCK @@ -13,6 +13,7 @@ EXPORTED_HEADERS = [ "NativeArray.h", "NativeCommon.h", "NativeMap.h", + "ReactMarker.h", "ReadableNativeArray.h", "ReadableNativeMap.h", "WritableNativeArray.h", diff --git a/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp b/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp new file mode 100644 index 00000000000000..c4680eedff2495 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp @@ -0,0 +1,62 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include "ReactMarker.h" +#include +#include +#include +#include + +namespace facebook { +namespace react { + +void JReactMarker::setLogPerfMarkerIfNeeded() { + static std::once_flag flag {}; + std::call_once(flag, [](){ + ReactMarker::logTaggedMarker = JReactMarker::logPerfMarker; + }); +} + +void JReactMarker::logMarker(const std::string& marker) { + static auto cls = javaClassStatic(); + static auto meth = cls->getStaticMethod("logMarker"); + meth(cls, marker); +} + +void JReactMarker::logMarker(const std::string& marker, const std::string& tag) { + static auto cls = javaClassStatic(); + static auto meth = cls->getStaticMethod("logMarker"); + meth(cls, marker, tag); +} + +void JReactMarker::logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) { + switch (markerId) { + case ReactMarker::RUN_JS_BUNDLE_START: + JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag); + break; + case ReactMarker::RUN_JS_BUNDLE_STOP: + JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag); + break; + case ReactMarker::CREATE_REACT_CONTEXT_STOP: + JReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); + break; + case ReactMarker::JS_BUNDLE_STRING_CONVERT_START: + JReactMarker::logMarker("loadApplicationScript_startStringConvert"); + break; + case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP: + JReactMarker::logMarker("loadApplicationScript_endStringConvert"); + break; + case ReactMarker::NATIVE_MODULE_SETUP_START: + JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag); + break; + case ReactMarker::NATIVE_MODULE_SETUP_STOP: + JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag); + break; + case ReactMarker::NATIVE_REQUIRE_START: + case ReactMarker::NATIVE_REQUIRE_STOP: + // These are not used on Android. + break; + } +} + +} +} diff --git a/ReactAndroid/src/main/jni/react/jni/ReactMarker.h b/ReactAndroid/src/main/jni/react/jni/ReactMarker.h new file mode 100644 index 00000000000000..7f918c77cbcf1d --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/ReactMarker.h @@ -0,0 +1,24 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include +#include +#include + +namespace facebook { +namespace react { + +class JReactMarker : public facebook::jni::JavaClass { +public: + static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; + static void setLogPerfMarkerIfNeeded(); + +private: + static void logMarker(const std::string& marker); + static void logMarker(const std::string& marker, const std::string& tag); + static void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag); +}; + +} +} From ff908aa9fc002c12398c4e3cab477bbe0a16d41a Mon Sep 17 00:00:00 2001 From: Alex Hernandez Date: Thu, 12 Oct 2017 15:28:25 -0700 Subject: [PATCH 19/32] Add helpful examples to Android test env validation failure messages. Summary: Hi there! I set up the repo because I want to do a little contribution to the Android side but ran into trouble [running tests](https://facebook.github.io/react-native/docs/contributing.html). The Android environment validation kept failing. While the messages were a little helpful, it would have saved me a bit of time and research if I had some helpful examples to copy-and-paste. In my case, I'm using Android SDK Tools on the command line. Hopefully, this will help others when setting up! Run `./scripts/run-android-local-unit-tests.sh` with each of the following to see the expanded help messages: * Make sure `$ANDROID_HOME/platforms/android-$MAJOR` is _not_ present. * Make sure `$ANDROID_HOME/build-tools/$BUILD_TOOLS_VERSION` is _not_ present. * Make sure `JAVA_HOME` is _not_ set and/or is _not_ in `PATH`. Closes https://github.com/facebook/react-native/pull/16222 Differential Revision: D6044875 Pulled By: ericnakagawa fbshipit-source-id: febacbd08fb632b349c352035f24eed891fbd154 --- scripts/validate-android-test-env.sh | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/scripts/validate-android-test-env.sh b/scripts/validate-android-test-env.sh index 7542e996d9ff5b..8b428e76fc4bdf 100755 --- a/scripts/validate-android-test-env.sh +++ b/scripts/validate-android-test-env.sh @@ -54,6 +54,11 @@ if [ ! -e "$PLATFORM_DIR" ]; then echo "Specifically, the directory $PLATFORM_DIR does not exist." echo "You probably need to specify the right version using the SDK Manager from within Android Studio." echo "See https://facebook.github.io/react-native/docs/getting-started.html for details." + echo "If you are using Android SDK Tools from the command line, you may need to run:" + echo + echo " sdkmanager \"platform-tools\" \"platform-tools;android-$MAJOR\"" + echo + echo "Check out https://developer.android.com/studio/command-line/sdkmanager.html for details." exit 1 fi @@ -64,6 +69,11 @@ if [ ! -e "$BT_DIR" ]; then echo "Specifically, the directory $BT_DIR does not exist." echo "You probably need to explicitly install the correct version of the Android SDK Build Tools from within Android Studio." echo "See https://facebook.github.io/react-native/docs/getting-started.html for details." + echo "If you are using Android SDK Tools from the command line, you may need to run:" + echo + echo " sdkmanager \"platform-tools\" \"build-tools;android-$BUILD_TOOLS_VERSION\"" + echo + echo "Check out https://developer.android.com/studio/command-line/sdkmanager.html for details." exit 1 fi @@ -80,7 +90,11 @@ if [ -n "$(which csrutil)" ]; then echo "See https://our.intern.facebook.com/intern/dex/installing-java-8/ for instructions on installing Java 8 on FB laptops." else echo "Check out http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html ." - echo "Be sure that you set JAVA_HOME and PATH correctly." + echo "Be sure that you set JAVA_HOME and PATH correctly in your .bashrc or equivalent. Example:" + echo + echo " export JAVA_HOME=path/to/java" + echo " export PATH=\$PATH:\$JAVA_HOME/bin" + echo fi echo "After installing Java, run 'buck kill' and 'buck clean'." exit 1 From 3826c14c0440e9830da4cbe7bf5aec2d2858a90c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ramos=20Ortiz?= Date: Thu, 12 Oct 2017 17:33:13 -0700 Subject: [PATCH 20/32] Configure a couple new bot integrations Summary: Closes https://github.com/facebook/react-native/pull/16344 Differential Revision: D6044166 Pulled By: hramos fbshipit-source-id: 651bec1c1d1afa974f5a7b285ccf8a77bf281d21 --- .github/no-response.yml | 9 +++++++++ .github/stale.yml | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 .github/no-response.yml create mode 100644 .github/stale.yml diff --git a/.github/no-response.yml b/.github/no-response.yml new file mode 100644 index 00000000000000..24203633432e73 --- /dev/null +++ b/.github/no-response.yml @@ -0,0 +1,9 @@ +# Configuration for probot-no-response - https://github.com/probot/no-response + +# Number of days of inactivity before an Issue is closed for lack of response +daysUntilClose: 14 +# Label requiring a response +responseRequiredLabel: Needs more information +# Comment to post when closing an Issue for lack of response. Set to `false` to disable +closeComment: > + Closing this issue as more information is needed to debug this and we have not heard back from the author. diff --git a/.github/stale.yml b/.github/stale.yml new file mode 100644 index 00000000000000..dc018b58604d55 --- /dev/null +++ b/.github/stale.yml @@ -0,0 +1,20 @@ +# Number of days of inactivity before an issue becomes stale +daysUntilStale: 60 +# Number of days of inactivity before a stale issue is closed +daysUntilClose: 7 +# Issues with these labels will never be considered stale +exemptLabels: + - Good first issue + - For Discussion + - Core Team +# Label to use when marking an issue as stale +staleLabel: Stale +# Comment to post when marking an issue as stale. Set to `false` to disable +markComment: > + This issue has been automatically marked as stale because it has not had + recent activity. It will be closed if no further activity occurs. + Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. + If you think this issue should definitely remain open, please let us know why. + Thank you for your contributions. +# Comment to post when closing a stale issue. Set to `false` to disable +closeComment: false From e60a2d0a530bc987322fd946435a4fe24e69a164 Mon Sep 17 00:00:00 2001 From: Max Sherman Date: Thu, 12 Oct 2017 19:19:16 -0700 Subject: [PATCH 21/32] Revert D5939688: [hermes][rn] Share logPerfMarker with hermes Differential Revision: D5939688 fbshipit-source-id: 37daeeaa01144472c26383106c88dd49bb99a37b --- .../main/jni/react/jni/AndroidJSCFactory.cpp | 50 ++++++++++++++- ReactAndroid/src/main/jni/react/jni/BUCK | 1 - .../src/main/jni/react/jni/ReactMarker.cpp | 62 ------------------- .../src/main/jni/react/jni/ReactMarker.h | 24 ------- 4 files changed, 48 insertions(+), 89 deletions(-) delete mode 100644 ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp delete mode 100644 ReactAndroid/src/main/jni/react/jni/ReactMarker.h diff --git a/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp b/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp index 0aba406fc24b86..52924b641362b4 100644 --- a/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp +++ b/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp @@ -13,7 +13,6 @@ #include "JSCPerfLogging.h" #include "JSLogging.h" -#include "ReactMarker.h" using namespace facebook::jni; @@ -22,6 +21,53 @@ namespace react { namespace { +class JReactMarker : public JavaClass { + public: + static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; + + static void logMarker(const std::string& marker) { + static auto cls = javaClassStatic(); + static auto meth = cls->getStaticMethod("logMarker"); + meth(cls, marker); + } + + static void logMarker(const std::string& marker, const std::string& tag) { + static auto cls = javaClassStatic(); + static auto meth = cls->getStaticMethod("logMarker"); + meth(cls, marker, tag); + } +}; + +void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) { + switch (markerId) { + case ReactMarker::RUN_JS_BUNDLE_START: + JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag); + break; + case ReactMarker::RUN_JS_BUNDLE_STOP: + JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag); + break; + case ReactMarker::CREATE_REACT_CONTEXT_STOP: + JReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); + break; + case ReactMarker::JS_BUNDLE_STRING_CONVERT_START: + JReactMarker::logMarker("loadApplicationScript_startStringConvert"); + break; + case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP: + JReactMarker::logMarker("loadApplicationScript_endStringConvert"); + break; + case ReactMarker::NATIVE_MODULE_SETUP_START: + JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag); + break; + case ReactMarker::NATIVE_MODULE_SETUP_STOP: + JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag); + break; + case ReactMarker::NATIVE_REQUIRE_START: + case ReactMarker::NATIVE_REQUIRE_STOP: + // These are not used on Android. + break; + } +} + ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) { auto jniEx = dynamic_cast(&ex); if (!jniEx) { @@ -80,7 +126,7 @@ namespace detail { void injectJSCExecutorAndroidPlatform() { // Inject some behavior into react/ - JReactMarker::setLogPerfMarkerIfNeeded(); + ReactMarker::logTaggedMarker = logPerfMarker; ExceptionHandling::platformErrorExtractor = extractJniError; JSCNativeHooks::loggingHook = nativeLoggingHook; JSCNativeHooks::nowHook = nativePerformanceNow; diff --git a/ReactAndroid/src/main/jni/react/jni/BUCK b/ReactAndroid/src/main/jni/react/jni/BUCK index 32878cae633d41..36f6faabae55df 100644 --- a/ReactAndroid/src/main/jni/react/jni/BUCK +++ b/ReactAndroid/src/main/jni/react/jni/BUCK @@ -13,7 +13,6 @@ EXPORTED_HEADERS = [ "NativeArray.h", "NativeCommon.h", "NativeMap.h", - "ReactMarker.h", "ReadableNativeArray.h", "ReadableNativeMap.h", "WritableNativeArray.h", diff --git a/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp b/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp deleted file mode 100644 index c4680eedff2495..00000000000000 --- a/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2004-present Facebook. All Rights Reserved. - -#include "ReactMarker.h" -#include -#include -#include -#include - -namespace facebook { -namespace react { - -void JReactMarker::setLogPerfMarkerIfNeeded() { - static std::once_flag flag {}; - std::call_once(flag, [](){ - ReactMarker::logTaggedMarker = JReactMarker::logPerfMarker; - }); -} - -void JReactMarker::logMarker(const std::string& marker) { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("logMarker"); - meth(cls, marker); -} - -void JReactMarker::logMarker(const std::string& marker, const std::string& tag) { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("logMarker"); - meth(cls, marker, tag); -} - -void JReactMarker::logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) { - switch (markerId) { - case ReactMarker::RUN_JS_BUNDLE_START: - JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag); - break; - case ReactMarker::RUN_JS_BUNDLE_STOP: - JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag); - break; - case ReactMarker::CREATE_REACT_CONTEXT_STOP: - JReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); - break; - case ReactMarker::JS_BUNDLE_STRING_CONVERT_START: - JReactMarker::logMarker("loadApplicationScript_startStringConvert"); - break; - case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP: - JReactMarker::logMarker("loadApplicationScript_endStringConvert"); - break; - case ReactMarker::NATIVE_MODULE_SETUP_START: - JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag); - break; - case ReactMarker::NATIVE_MODULE_SETUP_STOP: - JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag); - break; - case ReactMarker::NATIVE_REQUIRE_START: - case ReactMarker::NATIVE_REQUIRE_STOP: - // These are not used on Android. - break; - } -} - -} -} diff --git a/ReactAndroid/src/main/jni/react/jni/ReactMarker.h b/ReactAndroid/src/main/jni/react/jni/ReactMarker.h deleted file mode 100644 index 7f918c77cbcf1d..00000000000000 --- a/ReactAndroid/src/main/jni/react/jni/ReactMarker.h +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2004-present Facebook. All Rights Reserved. - -#pragma once - -#include -#include -#include - -namespace facebook { -namespace react { - -class JReactMarker : public facebook::jni::JavaClass { -public: - static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; - static void setLogPerfMarkerIfNeeded(); - -private: - static void logMarker(const std::string& marker); - static void logMarker(const std::string& marker, const std::string& tag); - static void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag); -}; - -} -} From 4a2d3e965360b8cb844df252cd8d2e342e5ea0c2 Mon Sep 17 00:00:00 2001 From: Paito Anderson Date: Fri, 13 Oct 2017 00:12:31 -0700 Subject: [PATCH 22/32] Fixed homogenous typo Summary: Closes https://github.com/facebook/react-native/pull/16285 Differential Revision: D6048989 Pulled By: hramos fbshipit-source-id: e6affb221961d14827ff655069571a3dc57197a1 --- Libraries/Lists/SectionList.js | 2 +- docs/UsingAScrollView.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Libraries/Lists/SectionList.js b/Libraries/Lists/SectionList.js index cbca922c77b650..775685131d6eaa 100644 --- a/Libraries/Lists/SectionList.js +++ b/Libraries/Lists/SectionList.js @@ -220,7 +220,7 @@ type DefaultProps = typeof defaultProps; * } * renderSectionHeader={({section}) =>
} - * sections={[ // homogenous rendering between sections + * sections={[ // homogeneous rendering between sections * {data: [...], title: ...}, * {data: [...], title: ...}, * {data: [...], title: ...}, diff --git a/docs/UsingAScrollView.md b/docs/UsingAScrollView.md index dbff13f201987f..94be9736c586b5 100644 --- a/docs/UsingAScrollView.md +++ b/docs/UsingAScrollView.md @@ -8,7 +8,7 @@ next: using-a-listview previous: handling-touches --- -The [ScrollView](docs/scrollview.html) is a generic scrolling container that can host multiple components and views. The scrollable items need not be homogenous, and you can scroll both vertically and horizontally (by setting the `horizontal` property). +The [ScrollView](docs/scrollview.html) is a generic scrolling container that can host multiple components and views. The scrollable items need not be homogeneous, and you can scroll both vertically and horizontally (by setting the `horizontal` property). This example creates a vertical `ScrollView` with both images and text mixed together. From 2fff445b13da114cc5238ad9064ed2100030d5b9 Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Fri, 13 Oct 2017 08:01:50 -0700 Subject: [PATCH 23/32] RN: Improve NativeEventEmitter Flow Types Reviewed By: fkgozali Differential Revision: D6050987 fbshipit-source-id: 1c911bed23f7d26800aafed4b7e7c30a1197f64b --- Libraries/EventEmitter/NativeEventEmitter.js | 24 ++++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/Libraries/EventEmitter/NativeEventEmitter.js b/Libraries/EventEmitter/NativeEventEmitter.js index be0de96f794946..5c979600295b54 100644 --- a/Libraries/EventEmitter/NativeEventEmitter.js +++ b/Libraries/EventEmitter/NativeEventEmitter.js @@ -14,18 +14,24 @@ const EventEmitter = require('EventEmitter'); const Platform = require('Platform'); const RCTDeviceEventEmitter = require('RCTDeviceEventEmitter'); + const invariant = require('fbjs/lib/invariant'); import type EmitterSubscription from 'EmitterSubscription'; +type NativeModule = { + +addListener: (eventType: string) => void, + +removeListeners: (count: number) => void, +}; + /** * Abstract base class for implementing event-emitting modules. This implements * a subset of the standard EventEmitter node module API. */ class NativeEventEmitter extends EventEmitter { - _nativeModule: Object; + _nativeModule: ?NativeModule; - constructor(nativeModule: Object) { + constructor(nativeModule: ?NativeModule) { super(RCTDeviceEventEmitter.sharedSubscriber); if (Platform.OS === 'ios') { invariant(nativeModule, 'Native module cannot be null.'); @@ -33,8 +39,12 @@ class NativeEventEmitter extends EventEmitter { } } - addListener(eventType: string, listener: Function, context: ?Object): EmitterSubscription { - if (Platform.OS === 'ios') { + addListener( + eventType: string, + listener: Function, + context: ?Object, + ): EmitterSubscription { + if (this._nativeModule != null) { this._nativeModule.addListener(eventType); } return super.addListener(eventType, listener, context); @@ -42,15 +52,15 @@ class NativeEventEmitter extends EventEmitter { removeAllListeners(eventType: string) { invariant(eventType, 'eventType argument is required.'); - if (Platform.OS === 'ios') { - const count = this.listeners(eventType).length; + const count = this.listeners(eventType).length; + if (this._nativeModule != null) { this._nativeModule.removeListeners(count); } super.removeAllListeners(eventType); } removeSubscription(subscription: EmitterSubscription) { - if (Platform.OS === 'ios') { + if (this._nativeModule != null) { this._nativeModule.removeListeners(1); } super.removeSubscription(subscription); From 9877c088558ad0c4ca9b313074e57c6eb370cce6 Mon Sep 17 00:00:00 2001 From: Trevor Brindle Date: Fri, 13 Oct 2017 13:26:38 -0700 Subject: [PATCH 24/32] add release notes/changelog requirement to PR Template Summary: grabbou has to write release notes for every release, and that's getting ridonc. Have you seen the changelog from 0.48? PR writers need to do their own, hopefully in a standardized format so it can be scripted. Just a PR template change, just proofreading needed. Please mention if have missed or added any extra Categories, Types, or Where columns. Here's a sample: [GENERAL] [ENHANCEMENT] [PULL_REQUEST_TEMPLATE.md] - added release notes/changelog requirement to PR Template Closes https://github.com/facebook/react-native/pull/15874 Differential Revision: D6054557 Pulled By: hramos fbshipit-source-id: 5741b98a0efdb1a6aaaeedaa292403b82ce713f6 --- .github/PULL_REQUEST_TEMPLATE.md | 30 ++++++++++++++++++++++++++++++ danger/dangerfile.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 05161b55a05169..126b18a9854ac3 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -9,8 +9,38 @@ Happy contributing! --> +## Motivation + (Write your motivation here.) ## Test Plan (Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!) + +## Release Notes + diff --git a/danger/dangerfile.js b/danger/dangerfile.js index 23ad03ff7dc6cc..13c1ed85d863d9 100644 --- a/danger/dangerfile.js +++ b/danger/dangerfile.js @@ -93,6 +93,35 @@ if (!includesTestPlan && !editsDocs) { markdown('@facebook-github-bot label Needs more information'); } +// Regex looks for given categories, types, a file/framework/component, and a message - broken into 4 capture groups +const releaseNotesRegex = /\[(ANDROID|CLI|DOCS|GENERAL|INTERNAL|IOS|TVOS|WINDOWS)\]\s*?\[(BREAKING|BUGFIX|ENHANCEMENT|FEATURE|MINOR)\]\s*?\[(.*)\]\s*?\-\s*?(.*)/ig; +const includesReleaseNotes = danger.github.pr.body.toLowerCase().includes('release notes'); +const correctlyFormattedReleaseNotes = releaseNotesRegex.test(danger.github.pr.body); +const releaseNotesCaptureGroups = releaseNotesRegex.exec(danger.github.pr.body); + +if (!includesReleaseNotes) { + const title = ':clipboard: Release Notes'; + const idea = 'This PR appears to be missing Release Notes.'; + warn(`${title} - ${idea}`); + markdown('@facebook-github-bot label Needs more information'); +} else if (!correctlyFormattedReleaseNotes) { + const title = ':clipboard: Release Notes'; + const idea = 'This PR may have incorrectly formatted Release Notes.'; + warn(`${title} - ${idea}`); + markdown('@facebook-github-bot label Needs more information'); +} else if (releaseNotesCaptureGroups) { + const category = releaseNotesCaptureGroups[1].toLowerCase(); + + // Use Release Notes to Tag PRs appropriately + if (category === 'ios' ){ + markdown('@facebook-github-bot label iOS'); + } + + if (category === 'android' ){ + markdown('@facebook-github-bot label Android'); + } +} + // Tags PRs that have been submitted by a core contributor. // TODO: Switch to using an actual MAINTAINERS file. const taskforce = fs.readFileSync('../bots/IssueCommands.txt', 'utf8').split('\n')[0].split(':')[1]; From f3b117abd06b2b8941be63d3e371efe09c114330 Mon Sep 17 00:00:00 2001 From: Max Sherman Date: Fri, 13 Oct 2017 14:00:46 -0700 Subject: [PATCH 25/32] Un-revert logMarker for hermes diff Reviewed By: mhorowitz Differential Revision: D6051880 fbshipit-source-id: 0ce4bbed9ba8579033ee5397ff6c0975b6886fb1 --- .../src/main/jni/react/jni/Android.mk | 1 + .../main/jni/react/jni/AndroidJSCFactory.cpp | 50 +-------------- ReactAndroid/src/main/jni/react/jni/BUCK | 1 + .../src/main/jni/react/jni/ReactMarker.cpp | 62 +++++++++++++++++++ .../src/main/jni/react/jni/ReactMarker.h | 24 +++++++ 5 files changed, 90 insertions(+), 48 deletions(-) create mode 100644 ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp create mode 100644 ReactAndroid/src/main/jni/react/jni/ReactMarker.h diff --git a/ReactAndroid/src/main/jni/react/jni/Android.mk b/ReactAndroid/src/main/jni/react/jni/Android.mk index 6f4744a7863abd..cc4cfbb5d446bd 100644 --- a/ReactAndroid/src/main/jni/react/jni/Android.mk +++ b/ReactAndroid/src/main/jni/react/jni/Android.mk @@ -22,6 +22,7 @@ LOCAL_SRC_FILES := \ NativeMap.cpp \ OnLoad.cpp \ ProxyExecutor.cpp \ + ReactMarker.cpp \ ReadableNativeArray.cpp \ ReadableNativeMap.cpp \ WritableNativeArray.cpp \ diff --git a/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp b/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp index 52924b641362b4..0aba406fc24b86 100644 --- a/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp +++ b/ReactAndroid/src/main/jni/react/jni/AndroidJSCFactory.cpp @@ -13,6 +13,7 @@ #include "JSCPerfLogging.h" #include "JSLogging.h" +#include "ReactMarker.h" using namespace facebook::jni; @@ -21,53 +22,6 @@ namespace react { namespace { -class JReactMarker : public JavaClass { - public: - static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; - - static void logMarker(const std::string& marker) { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("logMarker"); - meth(cls, marker); - } - - static void logMarker(const std::string& marker, const std::string& tag) { - static auto cls = javaClassStatic(); - static auto meth = cls->getStaticMethod("logMarker"); - meth(cls, marker, tag); - } -}; - -void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) { - switch (markerId) { - case ReactMarker::RUN_JS_BUNDLE_START: - JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag); - break; - case ReactMarker::RUN_JS_BUNDLE_STOP: - JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag); - break; - case ReactMarker::CREATE_REACT_CONTEXT_STOP: - JReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); - break; - case ReactMarker::JS_BUNDLE_STRING_CONVERT_START: - JReactMarker::logMarker("loadApplicationScript_startStringConvert"); - break; - case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP: - JReactMarker::logMarker("loadApplicationScript_endStringConvert"); - break; - case ReactMarker::NATIVE_MODULE_SETUP_START: - JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag); - break; - case ReactMarker::NATIVE_MODULE_SETUP_STOP: - JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag); - break; - case ReactMarker::NATIVE_REQUIRE_START: - case ReactMarker::NATIVE_REQUIRE_STOP: - // These are not used on Android. - break; - } -} - ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) { auto jniEx = dynamic_cast(&ex); if (!jniEx) { @@ -126,7 +80,7 @@ namespace detail { void injectJSCExecutorAndroidPlatform() { // Inject some behavior into react/ - ReactMarker::logTaggedMarker = logPerfMarker; + JReactMarker::setLogPerfMarkerIfNeeded(); ExceptionHandling::platformErrorExtractor = extractJniError; JSCNativeHooks::loggingHook = nativeLoggingHook; JSCNativeHooks::nowHook = nativePerformanceNow; diff --git a/ReactAndroid/src/main/jni/react/jni/BUCK b/ReactAndroid/src/main/jni/react/jni/BUCK index 36f6faabae55df..32878cae633d41 100644 --- a/ReactAndroid/src/main/jni/react/jni/BUCK +++ b/ReactAndroid/src/main/jni/react/jni/BUCK @@ -13,6 +13,7 @@ EXPORTED_HEADERS = [ "NativeArray.h", "NativeCommon.h", "NativeMap.h", + "ReactMarker.h", "ReadableNativeArray.h", "ReadableNativeMap.h", "WritableNativeArray.h", diff --git a/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp b/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp new file mode 100644 index 00000000000000..c4680eedff2495 --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/ReactMarker.cpp @@ -0,0 +1,62 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#include "ReactMarker.h" +#include +#include +#include +#include + +namespace facebook { +namespace react { + +void JReactMarker::setLogPerfMarkerIfNeeded() { + static std::once_flag flag {}; + std::call_once(flag, [](){ + ReactMarker::logTaggedMarker = JReactMarker::logPerfMarker; + }); +} + +void JReactMarker::logMarker(const std::string& marker) { + static auto cls = javaClassStatic(); + static auto meth = cls->getStaticMethod("logMarker"); + meth(cls, marker); +} + +void JReactMarker::logMarker(const std::string& marker, const std::string& tag) { + static auto cls = javaClassStatic(); + static auto meth = cls->getStaticMethod("logMarker"); + meth(cls, marker, tag); +} + +void JReactMarker::logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) { + switch (markerId) { + case ReactMarker::RUN_JS_BUNDLE_START: + JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag); + break; + case ReactMarker::RUN_JS_BUNDLE_STOP: + JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag); + break; + case ReactMarker::CREATE_REACT_CONTEXT_STOP: + JReactMarker::logMarker("CREATE_REACT_CONTEXT_END"); + break; + case ReactMarker::JS_BUNDLE_STRING_CONVERT_START: + JReactMarker::logMarker("loadApplicationScript_startStringConvert"); + break; + case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP: + JReactMarker::logMarker("loadApplicationScript_endStringConvert"); + break; + case ReactMarker::NATIVE_MODULE_SETUP_START: + JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag); + break; + case ReactMarker::NATIVE_MODULE_SETUP_STOP: + JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag); + break; + case ReactMarker::NATIVE_REQUIRE_START: + case ReactMarker::NATIVE_REQUIRE_STOP: + // These are not used on Android. + break; + } +} + +} +} diff --git a/ReactAndroid/src/main/jni/react/jni/ReactMarker.h b/ReactAndroid/src/main/jni/react/jni/ReactMarker.h new file mode 100644 index 00000000000000..7f918c77cbcf1d --- /dev/null +++ b/ReactAndroid/src/main/jni/react/jni/ReactMarker.h @@ -0,0 +1,24 @@ +// Copyright 2004-present Facebook. All Rights Reserved. + +#pragma once + +#include +#include +#include + +namespace facebook { +namespace react { + +class JReactMarker : public facebook::jni::JavaClass { +public: + static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;"; + static void setLogPerfMarkerIfNeeded(); + +private: + static void logMarker(const std::string& marker); + static void logMarker(const std::string& marker, const std::string& tag); + static void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag); +}; + +} +} From c0e6d415ce5e1e0d72cac58e68539f9a24ac7f31 Mon Sep 17 00:00:00 2001 From: Mike Grabowski Date: Fri, 13 Oct 2017 16:11:53 -0700 Subject: [PATCH 26/32] Migrate Travis over to Circle Summary: This pull request migrates Travis to Circle and pre-starts iOS simulators / tvOS ones as advised in documentation to speed up builds. It also uses Xcode 9.0 to build and test apps. Note that podspec test is failing and it's been failing for a while on Travis as well (since podspec has been changed to use Cxx bridge by default). I've notified few folks here of that and we are looking to fix this test, but it's not related to the scope of this PR. Also, previously, podspec tests were only runninng on master (disabled for PR builds) where I think it makes more sense to run them on PR builds as well (all in all, we want to prevent breakage before merging). That said, I've removed `if` check to make it run on all builds. Other small changes: - cache `node_modules` properly (previously defined as restore_cache and save_cache but not referenced in following jobs) Closes https://github.com/facebook/react-native/pull/16354 Differential Revision: D6054858 Pulled By: hramos fbshipit-source-id: 5165bef0985f4257febced14873be5bcb80b9f51 --- .circleci/config.yml | 198 +++++++++++++++++++++++++----------- .travis.yml | 49 --------- scripts/run-ci-e2e-tests.js | 2 +- 3 files changed, 139 insertions(+), 110 deletions(-) delete mode 100644 .travis.yml diff --git a/.circleci/config.yml b/.circleci/config.yml index cfc1eb70187d99..5a41bcb7fb256f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,39 +1,42 @@ aliases: - - &restore-cache + - &restore-node-cache keys: - - v1-dependencies-{{ .Branch }}-{{ checksum "package.json" }} + - v1-dependencies-{{ arch }}-{{ .Branch }}-{{ checksum "package.json" }} # Fallback in case checksum fails - - v1-dependencies-{{ .Branch }}- - - &save-cache + - v1-dependencies-{{ arch }}-{{ .Branch }}- + + - &save-node-cache paths: - node_modules - key: v1-dependencies-{{ .Branch }}-{{ checksum "package.json" }} + key: v1-dependencies-{{ arch }}-{{ .Branch }}-{{ checksum "package.json" }} - &restore-cache-website keys: - - v1-website-dependencies-{{ .Branch }}-{{ checksum "website/package.json" }} + - v1-website-dependencies-{{ arch }}-{{ .Branch }}-{{ checksum "website/package.json" }} # Fallback in case checksum fails - - v1-website-dependencies-{{ .Branch }}- + - v1-website-dependencies-{{ arch }}-{{ .Branch }}- + - &save-cache-website paths: - website/node_modules - key: v1-website-dependencies-{{ .Branch }}-{{ checksum "website/package.json" }} + key: v1-website-dependencies-{{ arch }}-{{ .Branch }}-{{ checksum "website/package.json" }} - &restore-cache-danger keys: - - v1-danger-dependencies-{{ .Branch }}-{{ checksum "danger/package.json" }} + - v1-danger-dependencies-{{ arch }}-{{ .Branch }}-{{ checksum "danger/package.json" }} # Fallback in case checksum fails - - v1-danger-dependencies-{{ .Branch }}- + - v1-danger-dependencies-{{ arch }}-{{ .Branch }}- + - &save-cache-danger paths: - danger/node_modules - key: v1-danger-dependencies-{{ .Branch }}-{{ checksum "danger/package.json" }} + key: v1-danger-dependencies-{{ arch }}-{{ .Branch }}-{{ checksum "danger/package.json" }} - &restore-cache-android-packages keys: - - v1-android-sdkmanager-packages-{{ checksum "scripts/circle-ci-android-setup.sh" }} + - v1-android-sdkmanager-packages-{{ arch }}-{{ checksum "scripts/circle-ci-android-setup.sh" }} # Fallback in case checksum fails - - v1-android-sdkmanager-packages- + - v1-android-sdkmanager-packages-{{ arch }}- - &save-cache-android-packages paths: - /opt/android/sdk/system-images/android-23 @@ -43,92 +46,151 @@ aliases: - /opt/android/sdk/platforms/android-19 - /opt/android/sdk/build-tools/23.0.1 - /opt/android/sdk/add-ons/addon-google_apis-google-23 - key: v1-android-sdkmanager-packages-{{ checksum "scripts/circle-ci-android-setup.sh" }} + key: v1-android-sdkmanager-packages-{{ arch }}-{{ checksum "scripts/circle-ci-android-setup.sh" }} - &restore-cache-ndk keys: - - v1-android-ndk-r10e-32-64 + - v1-android-ndk-{{ arch }}-r10e-32-64 + - &save-cache-ndk paths: - /opt/ndk - key: v1-android-ndk-r10e-32-64 + key: v1-android-ndk-{{ arch }}-r10e-32-64 - &restore-cache-buck-downloads keys: - - v1-buck-downloads-{{ .Branch }}-{{ checksum "ReactAndroid/build.gradle" }} + - v1-buck-downloads-{{ arch }}-{{ .Branch }}-{{ checksum "ReactAndroid/build.gradle" }} # Fallback in case checksum fails - - v1-buck-downloads-{{ .Branch }}- + - v1-buck-downloads-{{ arch }}-{{ .Branch }}- - &save-cache-buck-downloads paths: - "ReactAndroid/build/downloads" - key: v1-buck-downloads-{{ .Branch }}-{{ checksum "ReactAndroid/build.gradle" }} + key: v1-buck-downloads-{{ arch }}-{{ .Branch }}-{{ checksum "ReactAndroid/build.gradle" }} - &restore-cache-buck keys: - - v1-buck-v2017.09.04.02 + - v1-buck-{{ arch }}-v2017.09.04.02 - &save-cache-buck paths: - ~/buck - key: v1-buck-v2017.09.04.02 + key: v1-buck-{{ arch }}-v2017.09.04.02 - &restore-cache-watchman keys: - - v1-watchman-v4.9.0 + - v1-watchman-{{ arch }}-v4.9.0 - &save-cache-watchman paths: - ~/watchman - key: v1-watchman-v4.9.0 + key: v1-watchman-{{ arch }}-v4.9.0 + + - &install-node-dependencies + | + npm install --no-package-lock --no-spin --no-progress + + - &run-node-tests + | + npm test -- --maxWorkers=2 + npm run lint + npm run flow -- check defaults: &defaults working_directory: ~/react-native version: 2 jobs: - test-node-8: + + # Runs unit tests on Node 8 + test-js-node-8: <<: *defaults docker: - image: circleci/node:8 steps: - checkout - - run: npm install --no-package-lock - - run: | - npm test -- --maxWorkers=2 - npm run lint - npm run flow -- check -# eslint - doesn't run on non-PR builds - - run: - name: Analyze Code - command: | - if [ -n "$CIRCLE_PR_NUMBER" ]; then - npm install github@0.2.4 - cat <(echo eslint; npm run lint --silent -- --format=json; echo flow; npm run flow --silent -- check --json) | GITHUB_TOKEN="af6ef0d15709bc91d""06a6217a5a826a226fb57b7" CI_USER=$CIRCLE_PROJECT_USERNAME CI_REPO=$CIRCLE_PROJECT_REPONAME PULL_REQUEST_NUMBER=$CIRCLE_PR_NUMBER node bots/code-analysis-bot.js - else - echo "Skipping code analysis." - fi + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: *run-node-tests - test-node-6: + # Runs unit tests on Node 6 + test-js-node-6: <<: *defaults docker: - image: circleci/node:6.11.0 steps: - checkout - - run: npm install - - run: | - npm test -- --maxWorkers=2 - npm run lint - npm run flow -- check + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: *run-node-tests - test-node-4: + # Runs unit tests on Node 4 + test-js-node-4: <<: *defaults docker: - image: circleci/node:4.8.4 steps: - checkout - - run: npm install - - run: | - npm test -- --maxWorkers=2 - npm run lint - npm run flow -- check + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: *run-node-tests + + # Runs unit tests on iOS devices + test-objc-ios: + <<: *defaults + macos: + xcode: "9.0" + dependencies: + pre: + - xcrun instruments -w "iPhone 5s (10.3.1)" || true + steps: + - checkout + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: ./scripts/objc-test-ios.sh + + # Runs unit tests on tvOS devices + test-objc-tvos: + <<: *defaults + macos: + xcode: "9.0" + dependencies: + pre: + - xcrun instruments -w "Apple TV 1080p (10.0)" || true + steps: + - checkout + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: ./scripts/objc-test-tvos.sh + + # Runs end to end tests + test-objc-e2e: + <<: *defaults + macos: + xcode: "9.0" + dependencies: + pre: + - xcrun instruments -w "iPhone 5s (10.3.1)" || true + steps: + - checkout + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: node ./scripts/run-ci-e2e-tests.js --ios --js --retries 3; + + # Checks podspec + test-podspec: + <<: *defaults + macos: + xcode: "9.0" + steps: + - checkout + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache + - run: ./scripts/process-podspecs.sh test-website: <<: *defaults @@ -181,7 +243,9 @@ jobs: - image: circleci/node:8 steps: - checkout - - run: npm install --no-package-lock + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache - run: name: Build JavaScript Bundle command: node local-cli/cli.js bundle --max-workers 2 --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js @@ -258,7 +322,10 @@ jobs: command: | curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash - sudo apt-get install -y nodejs - - run: npm install + - restore-cache: *restore-node-cache + - run: *install-node-dependencies + - save-cache: *save-node-cache +# TODO: Install and use watchman to speed up builds # - restore-cache: *restore-cache-watchman # - run: # name: Install Watchman Dependencies @@ -322,11 +389,11 @@ jobs: - run: name: Build and Install Test APK command: source scripts/circle-ci-android-setup.sh && NO_BUCKD=1 retry3 buck install ReactAndroid/src/androidTest/buck-runner:instrumentation-tests --config build.threads=$BUILD_THREADS -# Failing test is expected +# TODO: Uncomment, test was already failing on Circle 1.0 # - run: # name: Run Installed APK with Tests # command: node ./scripts/run-android-ci-instrumentation-tests.js --retries 3 --path ./ReactAndroid/src/androidTest/java/com/facebook/react/tests --package com.facebook.react.tests -# Should be disabled pending on https://our.intern.facebook.com/intern/tasks?t=16912142 +# TODO: Should be disabled, pending on https://our.intern.facebook.com/intern/tasks?t=16912142 # - run: # name: Run Android End to End Tests # command: source scripts/circle-ci-android-setup.sh && retry3 node ./scripts/run-ci-e2e-tests.js --android --js --retries 2 @@ -338,7 +405,7 @@ jobs: mkdir -p ~/junit/ find . -type f -regex ".*/build/test-results/debug/.*xml" -exec cp {} ~/junit/ \; find . -type f -regex ".*/outputs/androidTest-results/connected/.*xml" -exec cp {} ~/junit/ \; -# TODO circle does not understand Buck's report, maybe need to transform xml slightly +# TODO: Circle does not understand Buck's report, maybe need to transform xml slightly # find . -type f -regex ".*/buck-out/gen/ReactAndroid/src/test/.*/.*xml" -exec cp {} ~/junit/ \; when: always - store_test_results: @@ -349,12 +416,14 @@ jobs: # Workflows enables us to run multiple jobs in parallel workflows: version: 2 + test_node: jobs: - - test-node-8 - - test-node-6 -# Node 4 tests are already failing on Circle 1.0 -# - test-node-4 + - test-js-node-8 + - test-js-node-6 +# TODO: Node 4 tests are already failing on Circle 1.0 +# - test-js-node-4 + website: jobs: - test-website @@ -366,6 +435,7 @@ workflows: only: - /.*-stable/ - master + test_android: jobs: - build-js-bundle: @@ -375,3 +445,11 @@ workflows: - test-android: requires: - build-js-bundle + + test_ios: + jobs: + - test-objc-ios + - test-objc-tvos +# TODO: Podspec tests are already failing on Travis +# - test-podspec + - test-objc-e2e diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 00abb8680236db..00000000000000 --- a/.travis.yml +++ /dev/null @@ -1,49 +0,0 @@ -language: objective-c - -osx_image: xcode8.3 - -install: - - nvm install 8 - - rm -Rf "${TMPDIR}/jest_preprocess_cache" - - brew install yarn --ignore-dependencies - - brew install watchman - - yarn install - -script: - - if [[ "$TEST_TYPE" = objc-ios ]]; then travis_retry travis_wait 30 ./scripts/objc-test-ios.sh test; fi - - if [[ "$TEST_TYPE" = objc-tvos ]]; then travis_retry travis_wait 30 ./scripts/objc-test-tvos.sh; fi - - if [[ "$TEST_TYPE" = e2e-objc ]]; then node ./scripts/run-ci-e2e-tests.js --ios --js --retries 3; fi - - if [[ ( "$TEST_TYPE" = podspecs ) && ( "$TRAVIS_PULL_REQUEST" = "false" ) ]]; then gem install cocoapods && travis_wait 30 ./scripts/process-podspecs.sh; fi - -cache: - - cocoapods - - yarn - -matrix: - - fast_finish: true # Fail the whole build as soon as one test type fails. Should help with Travis capacity issues (very long queues). - -# The order of these tests says which are more likely to run first and fail the whole build fast. -env: - - TEST_TYPE=objc-ios - - TEST_TYPE=podspecs - - TEST_TYPE=e2e-objc - - TEST_TYPE=objc-tvos - -branches: - only: - - master - - /^.*-stable$/ - -notifications: - email: - recipients: - - douglowder@mac.com # Doug Lowder built and maintains Apple TV specific code and wants to be notified about tvOS failures. - - eloy@artsy.net # Eloy Durán maintains the podspecs test and wants to be notified about failures. - on_failure: change - on_success: change - slack: - secure: oQL2C966v7/DtxNqfM7WowjY0R5mgLHR2qHkoucwK5iVrmaptnHr8fq01xlj7VT0kDwNLqT3n4+gtCviGw89lq71m3W76c8Pms/10jpjw+LwAfQPVizNw/Bx8MFNNmjDauK/auFxaybiLZupi7zd4xFGOZvScmFdfD4CAAp2OOA= - on_pull_requests: false - on_failure: change - on_success: change - webhooks: https://code.facebook.com/travis/webhook/ diff --git a/scripts/run-ci-e2e-tests.js b/scripts/run-ci-e2e-tests.js index 04ee69a2fbacf9..eb9bbe7c190f71 100644 --- a/scripts/run-ci-e2e-tests.js +++ b/scripts/run-ci-e2e-tests.js @@ -175,7 +175,7 @@ try { if (argv.tvos) { return exec('xcodebuild -destination "platform=tvOS Simulator,name=Apple TV 1080p,OS=10.0" -scheme EndToEndTest-tvOS -sdk appletvsimulator test | xcpretty && exit ${PIPESTATUS[0]}').code; } else { - return exec('xcodebuild -destination "platform=iOS Simulator,name=iPhone 5s,OS=10.0" -scheme EndToEndTest -sdk iphonesimulator test | xcpretty && exit ${PIPESTATUS[0]}').code; + return exec('xcodebuild -destination "platform=iOS Simulator,name=iPhone 5s,OS=10.3.1" -scheme EndToEndTest -sdk iphonesimulator test | xcpretty && exit ${PIPESTATUS[0]}').code; } }, numberOfRetries)) { From e268883fdc11b44d8e06616d045a342d0101eb45 Mon Sep 17 00:00:00 2001 From: Daniel Mueller Date: Fri, 13 Oct 2017 17:22:00 -0700 Subject: [PATCH 27/32] Improve support for unbundle feature Summary: unbundle is a useful feature, and it should be exposed. In order to get the most use out of we expose it as an option at build time in the Build Phase on XCode and the project.ext.react config in the build.gradle. Because it is best used with inline requires we add a section under performance that describes how inline requires can be implemented and how to use with the unbundling feature. Testing: - Added a section of the doc which explains how the feature can be enabled - Use the instructions, build a build on iOS + android (using release so that the bundle is created) and confirm that the bundle has the binary header information. Closes https://github.com/facebook/react-native/pull/15317 Differential Revision: D6054642 Pulled By: hramos fbshipit-source-id: 067f4d2f78d91215709bd3e3636f460bc2b17e99 --- docs/Performance.md | 262 ++++++++++++++++++++++++++++++++++ react.gradle | 19 ++- scripts/react-native-xcode.sh | 11 +- 3 files changed, 287 insertions(+), 5 deletions(-) diff --git a/docs/Performance.md b/docs/Performance.md index c23c06812d3382..4fc80edee1ef13 100644 --- a/docs/Performance.md +++ b/docs/Performance.md @@ -352,3 +352,265 @@ In the second scenario, you'll see something more like this: Notice that first the JS thread thinks for a bit, then you see some work done on the native modules thread, followed by an expensive traversal on the UI thread. There isn't an easy way to mitigate this unless you're able to postpone creating new UI until after the interaction, or you are able to simplify the UI you're creating. The react native team is working on a infrastructure level solution for this that will allow new UI to be created and configured off the main thread, allowing the interaction to continue smoothly. + +## Unbundling + inline requires + +If you have a large app you may want to consider unbundling and using inline requires. This is useful for apps that have a large number of screens which may not ever be opened during a typical usage of the app. Generally it is useful to apps that have large amounts of code that are not needed for a while after startup. For instance the app includes complicated profile screens or lesser used features, but most sessions only involve visiting the main screen of the app for updates. We can optimize the loading of the bundle by using the unbundle feature of the packager and requiring those features and screens inline (when they are actually used). + +### Loading JavaScript + +Before react-native can execute JS code, that code must be loaded into memory and parsed. With a standard bundle if you load a 50mb bundle, all 50mb must be loaded and parsed before any of it can be executed. The optimization behind unbundling is that you can load only the portion of the 50mb that you actually need at startup, and progressively load more of the bundle as those sections are needed. + +### Inline Requires + +Inline requires delay the requiring of a module or file until that file is actually needed. A basic example would look like this: + +#### VeryExpensive.js +``` +import React, { Component } from 'react'; +import { Text } from 'react-native'; +// ... import some very expensive modules + +// You may want to log at the file level to verify when this is happening +console.log('VeryExpensive component loaded'); + +export default class VeryExpensive extends Component { + // lots and lots of code + render() { + return Very Expensive Component; + } +} +``` + +#### Optimized.js +``` +import React, { Component } from 'react'; +import { TouchableOpacity, View, Text } from 'react-native'; + +let VeryExpensive = null; + +export default class Optimized extends Component { + state = { needsExpensive: false }; + + didPress = () => { + if (VeryExpensive == null) { + VeryExpensive = require('./VeryExpensive').default; + } + + this.setState(() => ({ + needsExpensive: true, + })); + }; + + render() { + return ( + + + Load + + {this.state.needsExpensive ? : null} + + ); + } +} +``` + +Even without unbundling inline requires can lead to startup time improvements, because the code within VeryExpensive.js will only execute once it is required for the first time. + +### Enable Unbundling + +On iOS unbundling will create a single indexed file that react native will load one module at a time. On Android, by default it will create a set of files for each module. You can force Android to create a single file, like iOS, but using multiple files can be more performant and requires less memory. + +Enable unbundling in Xcode by editing the build phase "Bundle React Native code and images". Before `../node_modules/react-native/packager/react-native-xcode.sh` add `export BUNDLE_COMMAND="unbundle"`: + +``` +export BUNDLE_COMMAND="unbundle" +export NODE_BINARY=node +../node_modules/react-native/packager/react-native-xcode.sh +``` + +On Android enable unbundling by editing your android/app/build.gradle file. Before the line `apply from: "../../node_modules/react-native/react.gradle"` add or amend the `project.ext.react` block: + +``` +project.ext.react = [ + bundleCommand: "unbundle", +] +``` + +Use the following lines on Android if you want to use a single indexed file: + +``` +project.ext.react = [ + bundleCommand: "unbundle", + extraPackagerArgs: ["--indexed-unbundle"] +] +``` + +### Configure Preloading and Inline Requires + +Now that we have unbundled our code, there is overhead for calling require. require now needs to send a message over the bridge when it encounters a module it has not loaded yet. This will impact startup the most, because that is where the largest number of require calls are likely to take place while the app loads the initial module. Luckily we can configure a portion of the modules to be preloaded. In order to do this, you will need to implement some form of inline require. + +### Adding a packager config file + +Create a folder in your project called packager, and create a single file named config.js. Add the following: + +``` +const config = { + getTransformOptions: () => { + return { + transform: { inlineRequires: true }, + }; + }, +}; + +module.exports = config; +``` + +In Xcode, in the build phase, include `export BUNDLE_CONFIG="packager/config.js"`. + +``` +export BUNDLE_COMMAND="unbundle" +export BUNDLE_CONFIG="packager/config.js" +export NODE_BINARY=node +../node_modules/react-native/packager/react-native-xcode.sh +``` + +Edit your android/app/build.gradle file to include `bundleConfig: "packager/config.js",`. + +``` +project.ext.react = [ + bundleCommand: "unbundle", + bundleConfig: "packager/config.js" +] +``` + +Finally, you can update "start" under "scripts" on your package.json to use the config: + +`"start": "node node_modules/react-native/local-cli/cli.js start --config ../../../../packager/config.js",` + +Start your package server with `npm start`. Note that when the dev packager is automatically launched via xcode and `react-native run-android`, etc, it does not use `npm start`, so it won't use the config. + +### Investigating the Loaded Modules + +In your root file (index.(ios|android).js) you can add the following after the initial imports: +``` +const modules = require.getModules(); +const moduleIds = Object.keys(modules); +const loadedModuleNames = moduleIds + .filter(moduleId => modules[moduleId].isInitialized) + .map(moduleId => modules[moduleId].verboseName); +const waitingModuleNames = moduleIds + .filter(moduleId => !modules[moduleId].isInitialized) + .map(moduleId => modules[moduleId].verboseName); + +// make sure that the modules you expect to be waiting are actually waiting +console.log( + 'loaded:', + loadedModuleNames.length, + 'waiting:', + waitingModuleNames.length +); + +// grab this text blob, and put it in a file named packager/moduleNames.js +console.log(`module.exports = ${JSON.stringify(loadedModuleNames.sort())};`); +``` + +When you run your app, you can look in the console and see how many modules have been loaded, and how many are waiting. You may want to read the moduleNames and see if there are any surprises. Note that inline requires are invoked the first time the imports are referenced. You may need to investigate and refactor to ensure only the modules you want are loaded on startup. Note that you can change the Systrace object on require to help debug problematic requires. + +``` +require.Systrace.beginEvent = (message) => { + if(message.includes(problematicModule)) { + throw new Error(); + } +} +``` + +Every app is different, but it may make sense to only load the modules you need for the very first screen. When you are satisified, put the output of the loadedModuleNames into a file named packager/moduleNames.js. + +### Transforming to Module Paths + +The loaded module names get us part of the way there, but we actually need absolute module paths, so the next script will set that up. Add `packager/generateModulePaths.js` to your project with the following: +``` +// @flow +/* eslint-disable no-console */ +const execSync = require('child_process').execSync; +const fs = require('fs'); +const moduleNames = require('./moduleNames'); + +const pjson = require('../package.json'); +const localPrefix = `${pjson.name}/`; + +const modulePaths = moduleNames.map(moduleName => { + if (moduleName.startsWith(localPrefix)) { + return `./${moduleName.substring(localPrefix.length)}`; + } + if (moduleName.endsWith('.js')) { + return `./node_modules/${moduleName}`; + } + try { + const result = execSync( + `grep "@providesModule ${moduleName}" $(find . -name ${moduleName}\\\\.js) -l` + ) + .toString() + .trim() + .split('\n')[0]; + if (result != null) { + return result; + } + } catch (e) { + return null; + } + return null; +}); + +const paths = modulePaths + .filter(path => path != null) + .map(path => `'${path}'`) + .join(',\n'); + +const fileData = `module.exports = [${paths}];`; + +fs.writeFile('./packager/modulePaths.js', fileData, err => { + if (err) { + console.log(err); + } + + console.log('Done'); +}); +``` + +You can run via `node packager/modulePaths.js`. + +This script attempts to map from the module names to module paths. Its not foolproof though, for instance, it ignores platform specific files (\*ios.js, and \*.android.js). However based on initial testing, it handles 95% of cases. When it runs, after some time it should complete and output a file named `packager/modulePaths.js`. It should contain paths to module files that are relative to your projects root. You can commit modulePaths.js to your repo so it is transportable. + +### Updating the config.js + +Returning to packager/config.js we should update it to use our newly generated modulePaths.js file. +``` +const modulePaths = require('./modulePaths'); +const resolve = require('path').resolve; +const fs = require('fs'); + +const config = { + getTransformOptions: () => { + const moduleMap = {}; + modulePaths.forEach(path => { + if (fs.existsSync(path)) { + moduleMap[resolve(path)] = true; + } + }); + return { + preloadedModules: moduleMap, + transform: { inlineRequires: { blacklist: moduleMap } }, + }; + }, +}; + +module.exports = config; +``` + +The preloadedModules entry in the config indicates which modules should be marked as preloaded by the unbundler. When the bundle is loaded, those modules are immediately loaded, before any requires have even executed. The blacklist entry indicates that those modules should not be required inline. Because they are preloaded, there is no performance benefit from using an inline require. In fact the javascript spends extra time resolving the inline require every time the imports are referenced. + +### Test and Measure Improvements + +You should now be ready to build your app using unbundling and inline requires. Make sure you measure the before and after startup times. diff --git a/react.gradle b/react.gradle index 109b0dab905590..64b2f02f8cbf26 100644 --- a/react.gradle +++ b/react.gradle @@ -5,6 +5,7 @@ def config = project.hasProperty("react") ? project.react : []; def cliPath = config.cliPath ?: "node_modules/react-native/local-cli/cli.js" def bundleAssetName = config.bundleAssetName ?: "index.android.bundle" def entryFile = config.entryFile ?: "index.android.js" +def bundleCommand = config.bundleCommand ?: "bundle" // because elvis operator def elvisFile(thing) { @@ -13,6 +14,7 @@ def elvisFile(thing) { def reactRoot = elvisFile(config.root) ?: file("../../") def inputExcludes = config.inputExcludes ?: ["android/**", "ios/**"] +def bundleConfig = config.bundleConfig ? "${reactRoot}/${config.bundleConfig}" : null ; void runBefore(String dependentTaskName, Task task) { Task dependentTask = tasks.findByPath(dependentTaskName); @@ -79,12 +81,21 @@ gradle.projectsEvaluated { // Set up dev mode def devEnabled = !(config."devDisabledIn${targetName}" || targetName.toLowerCase().contains("release")) + + def extraArgs = extraPackagerArgs; + + if (bundleConfig) { + extraArgs = extraArgs.clone() + extraArgs.add("--config"); + extraArgs.add(bundleConfig); + } + if (Os.isFamily(Os.FAMILY_WINDOWS)) { - commandLine("cmd", "/c", *nodeExecutableAndArgs, cliPath, "bundle", "--platform", "android", "--dev", "${devEnabled}", - "--reset-cache", "--entry-file", entryFile, "--bundle-output", jsBundleFile, "--assets-dest", resourcesDir, *extraPackagerArgs) + commandLine("cmd", "/c", *nodeExecutableAndArgs, cliPath, bundleCommand, "--platform", "android", "--dev", "${devEnabled}", + "--reset-cache", "--entry-file", entryFile, "--bundle-output", jsBundleFile, "--assets-dest", resourcesDir, *extraArgs) } else { - commandLine(*nodeExecutableAndArgs, cliPath, "bundle", "--platform", "android", "--dev", "${devEnabled}", - "--reset-cache", "--entry-file", entryFile, "--bundle-output", jsBundleFile, "--assets-dest", resourcesDir, *extraPackagerArgs) + commandLine(*nodeExecutableAndArgs, cliPath, bundleCommand, "--platform", "android", "--dev", "${devEnabled}", + "--reset-cache", "--entry-file", entryFile, "--bundle-output", jsBundleFile, "--assets-dest", resourcesDir, *extraArgs) } enabled config."bundleIn${targetName}" || diff --git a/scripts/react-native-xcode.sh b/scripts/react-native-xcode.sh index bfc419d1101748..121eb5f2fae874 100755 --- a/scripts/react-native-xcode.sh +++ b/scripts/react-native-xcode.sh @@ -70,6 +70,14 @@ fi [ -z "$CLI_PATH" ] && export CLI_PATH="$REACT_NATIVE_DIR/local-cli/cli.js" +[ -z "$BUNDLE_COMMAND" ] && BUNDLE_COMMAND="bundle" + +if [[ -z "$BUNDLE_CONFIG" ]]; then + CONFIG_ARG="" +else + CONFIG_ARG="--config $(pwd)/$BUNDLE_CONFIG" +fi + nodejs_not_found() { echo "error: Can't find '$NODE_BINARY' binary to build React Native bundle" >&2 @@ -105,7 +113,8 @@ fi BUNDLE_FILE="$DEST/main.jsbundle" -$NODE_BINARY "$CLI_PATH" bundle \ +$NODE_BINARY $CLI_PATH $BUNDLE_COMMAND \ + $CONFIG_ARG \ --entry-file "$ENTRY_FILE" \ --platform ios \ --dev $DEV \ From a59d157df4cc80b3a11e4a43c51adf56301416eb Mon Sep 17 00:00:00 2001 From: Arman Dezfuli-Arjomandi Date: Fri, 13 Oct 2017 17:36:42 -0700 Subject: [PATCH 28/32] Update Animated docs to mention potential issues with VirtualizedList Summary: Doc update to clarify how to prevent `Animated.loop` and other animations from pre-empting `VirtualizedList` rendering as discussed in #16092. Closes https://github.com/facebook/react-native/pull/16136 Differential Revision: D6057466 Pulled By: hramos fbshipit-source-id: 946bcde97b364c623b48ddaeb643309630c072c9 --- Libraries/Animated/src/AnimatedImplementation.js | 7 +++++-- docs/Animations.md | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Libraries/Animated/src/AnimatedImplementation.js b/Libraries/Animated/src/AnimatedImplementation.js index 3e32591f7c9e89..71d616026f0b57 100644 --- a/Libraries/Animated/src/AnimatedImplementation.js +++ b/Libraries/Animated/src/AnimatedImplementation.js @@ -827,8 +827,11 @@ module.exports = { /** * Loops a given animation continuously, so that each time it reaches the * end, it resets and begins again from the start. Can specify number of - * times to loop using the key 'iterations' in the config. Will loop without - * blocking the UI thread if the child animation is set to 'useNativeDriver'. + * times to loop using the key `iterations` in the config. Will loop without + * blocking the UI thread if the child animation is set to `useNativeDriver: true`. + * In addition, loops can prevent `VirtualizedList`-based components from rendering + * more rows while the animation is running. You can pass `isInteraction: false` in the + * child animation config to fix this. */ loop, diff --git a/docs/Animations.md b/docs/Animations.md index 54c844e6a8c8f3..05af3dc0da658e 100644 --- a/docs/Animations.md +++ b/docs/Animations.md @@ -360,6 +360,8 @@ things like `transform` and `opacity` will work, but flexbox and position proper When using `Animated.event`, it will only work with direct events and not bubbling events. This means it does not work with `PanResponder` but does work with things like `ScrollView#onScroll`. +When an animation is running, it can prevent `VirtualizedList` components from rendering more rows. If you need to run a long or looping animation while the user is scrolling through a list, you can use `isInteraction: false` in your animation's config to prevent this issue. + ### Bear in mind While using transform styles such as `rotateY`, `rotateX`, and others ensure the transform style `perspective` is in place. From 8b044dcb8088dd226298c92575d411119eb959dd Mon Sep 17 00:00:00 2001 From: Willem Fibbe Date: Fri, 13 Oct 2017 17:49:29 -0700 Subject: [PATCH 29/32] Update the `NetInfo.isConnected` example code so that it uses the new `connectionChange` event Summary: The first code block already uses the new `connectionChange` event instead of the deprecated `change` event, so change this example code block as well to use the new event. I came across this while upgrading my RN version. In the debug-console I saw a deprecation warning, despite I was using the example-code. Looking at the source, I saw the example code block still used the deprecated event, so update it to use the new one. Closes https://github.com/facebook/react-native/pull/16357 Differential Revision: D6054428 Pulled By: hramos fbshipit-source-id: 72ef1a79ece7494cda3773461a740dbbdf383e7e --- Libraries/Network/NetInfo.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Libraries/Network/NetInfo.js b/Libraries/Network/NetInfo.js index 864929e094649d..ae598179feab32 100644 --- a/Libraries/Network/NetInfo.js +++ b/Libraries/Network/NetInfo.js @@ -158,12 +158,12 @@ const _isConnectedSubscriptions = new Map(); * function handleFirstConnectivityChange(isConnected) { * console.log('Then, is ' + (isConnected ? 'online' : 'offline')); * NetInfo.isConnected.removeEventListener( - * 'change', + * 'connectionChange', * handleFirstConnectivityChange * ); * } * NetInfo.isConnected.addEventListener( - * 'change', + * 'connectionChange', * handleFirstConnectivityChange * ); * ``` From 0266b70379ff80a8ed002398cde1e640c2e7696b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ramos?= <165856+hramos@users.noreply.github.com> Date: Fri, 13 Oct 2017 20:30:13 -0700 Subject: [PATCH 30/32] Remove references to Travis Summary: Removing remaining references to Travis, which is removed in #16354. Test Plan None Closes https://github.com/facebook/react-native/pull/16364 Differential Revision: D6057590 Pulled By: hramos fbshipit-source-id: 574a5cbbddbc09b48307d70555901b6ab5940e40 --- CONTRIBUTING.md | 14 ++++++-------- README.md | 2 +- docs/Contributing.md | 14 ++++++-------- docs/Testing.md | 4 ++-- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b55d7aa0d1746b..ff6328c75cb2d3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -92,16 +92,15 @@ If you're only fixing a bug, it's fine to submit a pull request right away but w Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it. -**Before submitting a pull request**, please make sure the following is done: +Please make sure the following is done when submitting a pull request: 1. Fork [the repository](https://github.com/facebook/react-native) and create your branch from `master`. 2. Add the copyright notice to the top of any new files you've added. -3. Describe your [**test plan**](https://facebook.github.io/react-native/docs/contributing.html#test-plan) in your commit. -4. Ensure [**tests pass**](https://facebook.github.io/react-native/docs/contributing.html#contrinuous-integration-tests) on both Travis and Circle CI. -5. Make sure your code lints (`npm run lint`). -6. If you haven't already, [sign the CLA](https://code.facebook.com/cla). +3. Describe your [**test plan**](/react-native/docs/contributing.html#test-plan) in your pull request description. Make sure to [test your changes](/react-native/docs/testing.html)! +4. Make sure your code lints (`npm run lint`). +5. If you haven't already, [sign the CLA](https://code.facebook.com/cla). -All pull requests should be opened against the `master` branch. +All pull requests should be opened against the `master` branch. After opening your pull request, ensure [**all tests pass**](/react-native/docs/contributing.html#contrinuous-integration-tests) on Circle CI. If a test fails and you believe it is unrelated to your change, leave a comment on the pull request explaining why. > **Note:** It is not necessary to keep clicking `Merge master to your branch` on the PR page. You would want to merge master if there are conflicts or tests are failing. The Facebook-GitHub-Bot ultimately squashes all commits to a single one before merging your PR. @@ -117,9 +116,8 @@ See [What is a Test Plan?](https://medium.com/@martinkonicek/what-is-a-test-plan #### Continuous integration tests -Make sure all **tests pass** on both [Travis][travis] and [Circle CI][circle]. PRs that break tests are unlikely to be merged. Learn more about [testing your changes here](https://facebook.github.io/react-native/docs/testing.html). +Make sure all **tests pass** on [Circle CI][circle]. PRs that break tests are unlikely to be merged. Learn more about [testing your changes here](/react-native/docs/testing.html). -[travis]: https://travis-ci.org/facebook/react-native [circle]: http://circleci.com/gh/facebook/react-native #### Breaking changes diff --git a/README.md b/README.md index dd22409280c31d..4efc9ff6487edc 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# [React Native](https://facebook.github.io/react-native/) · [![Travis CI Status](https://travis-ci.org/facebook/react-native.svg?branch=master)](https://travis-ci.org/facebook/react-native) [![Circle CI Status](https://circleci.com/gh/facebook/react-native.svg?style=shield)](https://circleci.com/gh/facebook/react-native) [![npm version](https://badge.fury.io/js/react-native.svg)](https://badge.fury.io/js/react-native) [![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg)](CONTRIBUTING.md#pull-requests) +# [React Native](https://facebook.github.io/react-native/) · [![Circle CI Status](https://circleci.com/gh/facebook/react-native.svg?style=shield)](https://circleci.com/gh/facebook/react-native) [![npm version](https://badge.fury.io/js/react-native.svg)](https://badge.fury.io/js/react-native) [![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg)](CONTRIBUTING.md#pull-requests) Learn once, write anywhere: Build mobile apps with React. diff --git a/docs/Contributing.md b/docs/Contributing.md index a4b9e384968dde..664ff0658556b8 100644 --- a/docs/Contributing.md +++ b/docs/Contributing.md @@ -99,16 +99,15 @@ If you're only fixing a bug, it's fine to submit a pull request right away but w Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it. -**Before submitting a pull request**, please make sure the following is done: +Please make sure the following is done when submitting a pull request: 1. Fork [the repository](https://github.com/facebook/react-native) and create your branch from `master`. 2. Add the copyright notice to the top of any new files you've added. -3. Describe your [**test plan**](/react-native/docs/contributing.html#test-plan) in your commit. -4. Ensure [**tests pass**](/react-native/docs/contributing.html#contrinuous-integration-tests) on both Travis and Circle CI. -5. Make sure your code lints (`npm run lint`). -6. If you haven't already, [sign the CLA](https://code.facebook.com/cla). +3. Describe your [**test plan**](/react-native/docs/contributing.html#test-plan) in your pull request description. Make sure to [test your changes](/react-native/docs/testing.html)! +4. Make sure your code lints (`npm run lint`). +5. If you haven't already, [sign the CLA](https://code.facebook.com/cla). -All pull requests should be opened against the `master` branch. +All pull requests should be opened against the `master` branch. After opening your pull request, ensure [**all tests pass**](/react-native/docs/contributing.html#contrinuous-integration-tests) on Circle CI. If a test fails and you believe it is unrelated to your change, leave a comment on the pull request explaining why. > **Note:** It is not necessary to keep clicking `Merge master to your branch` on the PR page. You would want to merge master if there are conflicts or tests are failing. The Facebook-GitHub-Bot ultimately squashes all commits to a single one before merging your PR. @@ -124,9 +123,8 @@ See [What is a Test Plan?](https://medium.com/@martinkonicek/what-is-a-test-plan #### Continuous integration tests -Make sure all **tests pass** on both [Travis][travis] and [Circle CI][circle]. PRs that break tests are unlikely to be merged. Learn more about [testing your changes here](/react-native/docs/testing.html). +Make sure all **tests pass** on [Circle CI][circle]. PRs that break tests are unlikely to be merged. Learn more about [testing your changes here](/react-native/docs/testing.html). -[travis]: https://travis-ci.org/facebook/react-native [circle]: http://circleci.com/gh/facebook/react-native #### Breaking changes diff --git a/docs/Testing.md b/docs/Testing.md index 6b7488572f1f81..55050e5401ef61 100644 --- a/docs/Testing.md +++ b/docs/Testing.md @@ -10,7 +10,7 @@ previous: maintainers This document is about testing your changes to React Native as a [contributor](docs/contributing.html). If you're interested in testing a React Native app, check out the [React Native Tutorial](http://facebook.github.io/jest/docs/tutorial-react-native.html) on the Jest website. -The React Native repo has several tests you can run to verify you haven't caused a regression with your PR. These tests are run with the [Travis](https://travis-ci.org/facebook/react-native/builds) and [Circle](https://circleci.com/gh/facebook/react-native) continuous integration systems, which will automatically annotate pull requests with the test results. +The React Native repo has several tests you can run to verify you haven't caused a regression with your PR. These tests are run using [Circle](https://circleci.com/gh/facebook/react-native), a continuous integration system. Circle will automatically annotate pull requests with the test results. Whenever you are fixing a bug or adding new functionality to React Native, you should add a test that covers it. Depending on the change you're making, there are different types of tests that may be appropriate. @@ -110,7 +110,7 @@ You can run integration tests locally with cmd+U in the IntegrationTest and RNTe A common type of integration test is the snapshot test. These tests render a component, and verify snapshots of the screen against reference images using `TestModule.verifySnapshot()`, using the [`FBSnapshotTestCase`](https://github.com/facebook/ios-snapshot-test-case) library behind the scenes. Reference images are recorded by setting `recordMode = YES` on the `RCTTestRunner`, then running the tests. Snapshots will differ slightly between 32 and 64 bit, and various OS versions, so it's recommended that you enforce tests are run with the correct configuration. It's also highly recommended that all network data be mocked out, along with other potentially troublesome dependencies. See [`SimpleSnapshotTest`](https://github.com/facebook/react-native/blob/master/IntegrationTests/SimpleSnapshotTest.js) for a basic example. -If you make a change that affects a snapshot test in a PR, such as adding a new example case to one of the examples that is snapshotted, you'll need to re-record the snapshot reference image. To do this, simply change to `_runner.recordMode = YES;` in [RNTester/RNTesterSnapshotTests.m](https://github.com/facebook/react-native/blob/master/RNTester/RNTesterIntegrationTests/RNTesterSnapshotTests.m#L42), re-run the failing tests, then flip record back to `NO` and submit/update your PR and wait to see if the Travis build passes. +If you make a change that affects a snapshot test in a PR, such as adding a new example case to one of the examples that is snapshotted, you'll need to re-record the snapshot reference image. To do this, simply change to `_runner.recordMode = YES;` in [RNTester/RNTesterSnapshotTests.m](https://github.com/facebook/react-native/blob/master/RNTester/RNTesterIntegrationTests/RNTesterSnapshotTests.m#L42), re-run the failing tests, then flip record back to `NO` and submit/update your PR and wait to see if the Circle build passes. ## Apple TV From 3b7067a62ddf8dace0fd2e197718cf435a246a83 Mon Sep 17 00:00:00 2001 From: Marcin Dobosz Date: Fri, 13 Oct 2017 20:38:13 -0700 Subject: [PATCH 31/32] Partial list of unsupported TextInput styles Summary: References #7070 Docs are incomplete, start filling them out N/A Closes https://github.com/facebook/react-native/pull/16346 Differential Revision: D6057501 Pulled By: hramos fbshipit-source-id: c30d3369fa1a73ef6a93c2ed8f8c53af5a1af7ee --- Libraries/Components/TextInput/TextInput.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Libraries/Components/TextInput/TextInput.js b/Libraries/Components/TextInput/TextInput.js index 9b96c0d162a5b5..f5ad6dcb5ccc4c 100644 --- a/Libraries/Components/TextInput/TextInput.js +++ b/Libraries/Components/TextInput/TextInput.js @@ -533,7 +533,17 @@ const TextInput = createReactClass({ */ blurOnSubmit: PropTypes.bool, /** - * Note that not all Text styles are supported, + * Note that not all Text styles are supported, an incomplete list of what is not supported includes: + * + * - `borderLeftWidth` + * - `borderTopWidth` + * - `borderRightWidth` + * - `borderBottomWidth` + * - `borderTopLeftRadius` + * - `borderTopRightRadius` + * - `borderBottomRightRadius` + * - `borderBottomLeftRadius` + * * see [Issue#7070](https://github.com/facebook/react-native/issues/7070) * for more detail. * From 720a99a89033a5e26226637625c65cbcb5249fe5 Mon Sep 17 00:00:00 2001 From: "glevi@fb.com" Date: Fri, 13 Oct 2017 21:23:39 -0700 Subject: [PATCH 32/32] @allow-large-files Deploy Flow v0.57.2 Reviewed By: samwgoldman Differential Revision: D6058747 fbshipit-source-id: 8dd289a7156be82715abfd66755724e7d916c4e5 --- .flowconfig | 6 +++--- local-cli/templates/HelloWorld/_flowconfig | 6 +++--- package.json | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.flowconfig b/.flowconfig index 8556171249fdaa..15ea5d95a5ea41 100644 --- a/.flowconfig +++ b/.flowconfig @@ -46,12 +46,12 @@ suppress_type=$FlowFixMeProps suppress_type=$FlowFixMeState suppress_type=$FixMe -suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*[react_native_oss|react_native_fb][a-z,_]*\\)?)\\) -suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*[react_native_oss|react_native_fb][a-z,_]*\\)?)\\)?:? #[0-9]+ +suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(5[0-7]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*[react_native_oss|react_native_fb][a-z,_]*\\)?)\\) +suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(5[0-7]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*[react_native_oss|react_native_fb][a-z,_]*\\)?)\\)?:? #[0-9]+ suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError unsafe.enable_getters_and_setters=true [version] -^0.56.0 +^0.57.0 diff --git a/local-cli/templates/HelloWorld/_flowconfig b/local-cli/templates/HelloWorld/_flowconfig index 1ac2a7be59d650..2f13324849ae0a 100644 --- a/local-cli/templates/HelloWorld/_flowconfig +++ b/local-cli/templates/HelloWorld/_flowconfig @@ -37,12 +37,12 @@ suppress_type=$FlowFixMeProps suppress_type=$FlowFixMeState suppress_type=$FixMe -suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\) -suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(5[0-6]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\)?:? #[0-9]+ +suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(>=0\\.\\(5[0-7]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\) +suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(>=0\\.\\(5[0-7]\\|[1-4][0-9]\\|[0-9]\\).[0-9]\\)? *\\(site=[a-z,_]*react_native[a-z,_]*\\)?)\\)?:? #[0-9]+ suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError unsafe.enable_getters_and_setters=true [version] -^0.56.0 +^0.57.0 diff --git a/package.json b/package.json index 8d03b4366620a7..6b94afb6e8d049 100644 --- a/package.json +++ b/package.json @@ -198,7 +198,7 @@ "eslint-plugin-flowtype": "^2.33.0", "eslint-plugin-prettier": "2.1.1", "eslint-plugin-react": "^7.2.1", - "flow-bin": "^0.56.0", + "flow-bin": "^0.57.0", "jest": "^21", "prettier": "1.7.0", "react": "16.0.0",