Remove @providesModule from all modules #18995

rubennorte · 2018-04-23T16:34:06Z

This PR removes the need for having the @providesModule tags in all the modules in the repository.

It configures Flow, Jest and Metro to get the module names from the filenames (Libraries/Animated/src/nodes/AnimatedInterpolation.js => AnimatedInterpolation)

Test Plan

Checked the Flow configuration by running flow on the project root (no errors):

yarn flow

Checked the Jest configuration by running the tests with a clean cache:

yarn jest --clearCache && yarn test

Checked the Metro configuration by starting the server with a clean cache and requesting some bundles:

yarn run start --reset-cache
curl 'localhost:8081/IntegrationTests/AccessibilityManagerTest.bundle?platform=android'
curl 'localhost:8081/Libraries/Alert/Alert.bundle?platform=ios'

Release Notes

[INTERNAL] [FEATURE] [All] - Removed @providesModule from all modules and configured tools.

pull-bot · 2018-04-23T16:36:55Z

	Warnings
⚠️	🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.
⚠️	❗ Big PR - This PR is extremely unlikely to get reviewed because it touches 638 lines.

Generated by 🚫 dangerJS

facebook-github-bot

@rubennorte has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

@rubennorte has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

chirag04 · 2018-05-08T14:54:03Z

jest/hasteImpl.js

+    return false;
+  }
+
+  if (!filePath.startsWith(ROOT)) {


hey @rubennorte e2e tests are failing right now and my guess is this might be one cause. Refer: https://circleci.com/gh/facebook/react-native/42132?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link.

I'm seeing ReferenceError: SHA-1 for file /private/tmp/react-native-9ke7hCEE/EndToEndTest/node_modules/fbjs/lib/invariant.js is not computed. I might be completely wrong but looks like we don't handle haste modules in the node_modules directory?

Hi @chirag04, I don't think that specific error is caused by the @providesModule change, but a bad integration between Metro and Watchman. @mjesun or @rafeca can you please confirm that?

Hi, yes, that might be. jest-haste-map is not computing SHA-1s for node_modules; I'm going to put up a fix for this.

Hi @mjesun! When might that fix be ready? It's keeping us from running Android tests.

Hi @hramos 🙂 Mentioned you in D8223545 😉

rozele · 2018-05-18T19:39:24Z

@rubennorte - can you explain how metro is configured by this PR? Does metro use the .flowconfig?

rubennorte · 2018-05-22T15:03:05Z

@rozele it has its own configuration for that:

react-native/local-cli/core/index.js

Line 73 in aba4ec0

hasteImplModulePath: require.resolve('../../jest/hasteImpl'),

The change is at the end of this PR.

rozele · 2018-05-22T15:49:13Z

@rubennorte - as far as I can tell, this will break react-native-windows because that haste config does not strip .windows.js platform extensions. We need to ensure there's a way to make bundling react-native-windows modules work without customizing rn-cli.config.js. We need a fully automated way of installing react-native-windows and having things work "out of the box".

cc @hramos @axemclion @vincentriemer @matthargett

rozele · 2018-05-22T15:59:59Z

What's the point of removing @providesModule anyway? It's actually a really nice hook for plugin platforms to override core behavior. It has a solid, and very clear use case.

rozele · 2018-05-22T16:00:56Z

Also, not only does this not strip .windows.js, but it also does not include ..\..\react-native-windows in the "whitelist" for haste eligible modules.

vincentriemer · 2018-05-22T16:18:50Z

Just chiming in to say this will likely break react-native-dom as well

mjesun · 2018-05-22T16:52:40Z

@rozele @vincentriemer We haven't removed support for @providesModule; we've just removed duplicated information: for a very long time we've been enforcing filenames to match with the @providesModule value, up to the point where the annotation became useless.

In fact, the change speeds up crawling process by removing the need - in certain cases - of opening the file to check the annotation, since the filename contains the same information. Haste maps should work the same way they've been working so far.

rozele · 2018-05-22T17:07:18Z

@mjesun - Thanks for clarifying.

rozele · 2018-06-28T14:58:51Z

@mjesun - hoping to get some clarity on your last response. I assumed when you said "we haven't removed support for @providesModule, you implied that I would still be able to override haste modules across NPM packages. This is how react-native-windows works, as there are many core react-native modules that need to be modified slightly to get things working on UWP. Is this an accurate assumption? Currently, this behavior is not working on react-native 0.56 RC, so I'm trying to figure out if this is "by design" (i.e., no more cross-package haste overrides) or if it's some other bug.

mjesun · 2018-06-28T15:03:45Z

Sorry, that's a FB "deviation" of the language, where we indistinctly use @providesModule to refer both to Haste and @providesModule itself. What was removed was the @providesModule annotations, but not the Haste support.

Haste is the ability of referring to a file from a require call just by its name, instead of its relative or absolute path. As long as your View file is called View.*, and Haste is instructed to read that, everything should still work.

rozele · 2018-06-28T15:23:51Z

Okay - that's good to know @mjesun. Sounds like the experience I'm currently having with metro is definitely a "bug" as opposed to a new limitation of the package then. I've followed up with bug reports here and on metro:
#19953
facebook/metro#188

Summary: This PR removes the need for having the `providesModule` tags in all the modules in the repository. It configures Flow, Jest and Metro to get the module names from the filenames (`Libraries/Animated/src/nodes/AnimatedInterpolation.js` => `AnimatedInterpolation`) * Checked the Flow configuration by running flow on the project root (no errors): ``` yarn flow ``` * Checked the Jest configuration by running the tests with a clean cache: ``` yarn jest --clearCache && yarn test ``` * Checked the Metro configuration by starting the server with a clean cache and requesting some bundles: ``` yarn run start --reset-cache curl 'localhost:8081/IntegrationTests/AccessibilityManagerTest.bundle?platform=android' curl 'localhost:8081/Libraries/Alert/Alert.bundle?platform=ios' ``` [INTERNAL] [FEATURE] [All] - Removed providesModule from all modules and configured tools. Closes facebook#18995 Reviewed By: mjesun Differential Revision: D7729509 Pulled By: rubennorte fbshipit-source-id: 892f760a05ce1fddb088ff0cd2e97e521fb8e825

Summary: This PR removes the need for having the `providesModule` tags in all the modules in the repository. It configures Flow, Jest and Metro to get the module names from the filenames (`Libraries/Animated/src/nodes/AnimatedInterpolation.js` => `AnimatedInterpolation`) * Checked the Flow configuration by running flow on the project root (no errors): ``` yarn flow ``` * Checked the Jest configuration by running the tests with a clean cache: ``` yarn jest --clearCache && yarn test ``` * Checked the Metro configuration by starting the server with a clean cache and requesting some bundles: ``` yarn run start --reset-cache curl 'localhost:8081/IntegrationTests/AccessibilityManagerTest.bundle?platform=android' curl 'localhost:8081/Libraries/Alert/Alert.bundle?platform=ios' ``` [INTERNAL] [FEATURE] [All] - Removed providesModule from all modules and configured tools. Closes facebook/react-native#18995 Reviewed By: mjesun Differential Revision: D7729509 Pulled By: rubennorte fbshipit-source-id: 892f760a05ce1fddb088ff0cd2e97e521fb8e825

Summary: This PR removes the need for having the `providesModule` tags in all the modules in the repository. It configures Flow, Jest and Metro to get the module names from the filenames (`Libraries/Animated/src/nodes/AnimatedInterpolation.js` => `AnimatedInterpolation`) * Checked the Flow configuration by running flow on the project root (no errors): ``` yarn flow ``` * Checked the Jest configuration by running the tests with a clean cache: ``` yarn jest --clearCache && yarn test ``` * Checked the Metro configuration by starting the server with a clean cache and requesting some bundles: ``` yarn run start --reset-cache curl 'localhost:8081/IntegrationTests/AccessibilityManagerTest.bundle?platform=android' curl 'localhost:8081/Libraries/Alert/Alert.bundle?platform=ios' ``` [INTERNAL] [FEATURE] [All] - Removed providesModule from all modules and configured tools. Closes facebook#18995 Reviewed By: mjesun Differential Revision: D7729509 Pulled By: rubennorte fbshipit-source-id: 892f760a05ce1fddb088ff0cd2e97e521fb8e825

Summary: This PR removes the need for having the `providesModule` tags in all the modules in the repository. It configures Flow, Jest and Metro to get the module names from the filenames (`Libraries/Animated/src/nodes/AnimatedInterpolation.js` => `AnimatedInterpolation`) * Checked the Flow configuration by running flow on the project root (no errors): ``` yarn flow ``` * Checked the Jest configuration by running the tests with a clean cache: ``` yarn jest --clearCache && yarn test ``` * Checked the Metro configuration by starting the server with a clean cache and requesting some bundles: ``` yarn run start --reset-cache curl 'localhost:8081/IntegrationTests/AccessibilityManagerTest.bundle?platform=android' curl 'localhost:8081/Libraries/Alert/Alert.bundle?platform=ios' ``` [INTERNAL] [FEATURE] [All] - Removed providesModule from all modules and configured tools. Closes facebook/react-native#18995 Reviewed By: mjesun Differential Revision: D7729509 Pulled By: rubennorte fbshipit-source-id: 892f760a05ce1fddb088ff0cd2e97e521fb8e825

rubennorte requested review from grabbou, hramos, Kureev and shergin as code owners April 23, 2018 16:34

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 23, 2018

react-native-bot added the 📋Release Notes label Apr 23, 2018

react-native-bot added the 📋Large PR ‼️ label Apr 23, 2018

rubennorte force-pushed the remove-providesmodule branch from b3ba289 to dab854c Compare April 23, 2018 16:44

rubennorte added 2 commits April 23, 2018 17:49

Configured Flow, Jest and Metro to use module names from filenames

8219ce3

Removed @providesModule from all the modules in the repo

aac0c68

rubennorte force-pushed the remove-providesmodule branch from dab854c to aac0c68 Compare April 23, 2018 16:49

facebook-github-bot reviewed Apr 23, 2018

View reviewed changes

rubennorte changed the title ~~Removed @providesModule from all modules~~ Remove @providesModule from all modules Apr 24, 2018

Re-signed generated modules

382e3ee

facebook-github-bot reviewed Apr 24, 2018

View reviewed changes

facebook-github-bot closed this in d5e9e55 Apr 25, 2018

chirag04 reviewed May 8, 2018

View reviewed changes

This was referenced May 23, 2018

[CI] Fix test_android failures, May 2018 #19309

Closed

0.56.x (latest: 0.56.0) react-native-community/releases#15

Closed

jhen0409 mentioned this pull request Jul 6, 2018

[RN 0.56] Context menu functions are broken jhen0409/react-native-debugger#244

Closed

thomas-gilbert mentioned this pull request Sep 17, 2018

RN 0.56 & 0.57 - providesModule no longer works #21152

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove @providesModule from all modules #18995

Remove @providesModule from all modules #18995

rubennorte commented Apr 23, 2018

pull-bot commented Apr 23, 2018 •

edited

Loading

facebook-github-bot left a comment

facebook-github-bot left a comment

chirag04 May 8, 2018

rubennorte May 22, 2018

mjesun May 22, 2018

hramos May 31, 2018

mjesun May 31, 2018

rozele commented May 18, 2018

rubennorte commented May 22, 2018 •

edited

Loading

rozele commented May 22, 2018 •

edited

Loading

rozele commented May 22, 2018

rozele commented May 22, 2018

vincentriemer commented May 22, 2018

mjesun commented May 22, 2018

rozele commented May 22, 2018

rozele commented Jun 28, 2018

mjesun commented Jun 28, 2018

rozele commented Jun 28, 2018

Remove @providesModule from all modules #18995

Remove @providesModule from all modules #18995

Conversation

rubennorte commented Apr 23, 2018

Test Plan

Release Notes

pull-bot commented Apr 23, 2018 • edited Loading

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

chirag04 May 8, 2018

Choose a reason for hiding this comment

rubennorte May 22, 2018

Choose a reason for hiding this comment

mjesun May 22, 2018

Choose a reason for hiding this comment

hramos May 31, 2018

Choose a reason for hiding this comment

mjesun May 31, 2018

Choose a reason for hiding this comment

rozele commented May 18, 2018

rubennorte commented May 22, 2018 • edited Loading

rozele commented May 22, 2018 • edited Loading

rozele commented May 22, 2018

rozele commented May 22, 2018

vincentriemer commented May 22, 2018

mjesun commented May 22, 2018

rozele commented May 22, 2018

rozele commented Jun 28, 2018

mjesun commented Jun 28, 2018

rozele commented Jun 28, 2018

pull-bot commented Apr 23, 2018 •

edited

Loading

rubennorte commented May 22, 2018 •

edited

Loading

rozele commented May 22, 2018 •

edited

Loading