status-im
/
react-native
mirror of https://github.com/status-im/react-native.git
2
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix @providesModule not being ignored properly 

Summary:
There's quite a bit of code scattered around the packager regarding ignoring the `providesModule` Haste pragma in any file that isn't in `react-native`, `react-tools` or `parse`. There is even a (passing) test case.

However, there's an edge case.

Take, for example, `fbjs`. It has a module inside of it called `ErrorUtils`. `react-relay` requires this file normally, in Common.JS style, by doing `require('fbjs/libs/ErrorUtils')`. But when `react-native` attempts to require `ErrorUtils` using the HasteModule format (in it's JavaScript initialization), it resolves the `fbjs` `ErrorUtils` module, instead of RN's `ErrorUtils`.

This happens, it turns out, because when a module is read (in `Module._read`), it's not caring about whether or not it should pay attention to `providesModule`, and is just assigning the `providesModule` value as the id of the module no matter what. Then when `Module.getName` is called, it will always use that `data.id` that was set, thus creating the wrong dependency tree.

This
Closes https://github.com/facebook/react-native/pull/3625

Reviewed By: svcscm

Differential Revision: D2632317

Pulled By: vjeux

fb-gh-sync-id: efd8066eaf6f18fcf79698beab36cab90bf5cd6d
This commit is contained in:
Adam Miskiewicz 2015-12-24 08:31:17 -08:00 committed by facebook-github-bot-5
parent 59885911e3
commit 6cec263ca3
11 changed files with 140 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packager/react-packager/src/DependencyResolver/AssetModule.js vendored
View File

@ -25,7 +25,7 @@ class AssetModule extends Module {
return Promise.resolve([]);
}
_read() {
read() {
return Promise.resolve({});
}

4
packager/react-packager/src/DependencyResolver/DependencyGraph/Helpers.js → packager/react-packager/src/DependencyResolver/DependencyGraph/DependencyGraphHelpers.js vendored
View File

@ -10,7 +10,7 @@
const path = require('path');
class Helpers {
class DependencyGraphHelpers {
constructor({ providesModuleNodeModules, assetExts }) {
this._providesModuleNodeModules = providesModuleNodeModules;
this._assetExts = assetExts;
@ -46,4 +46,4 @@ class Helpers {
}
}
module.exports = Helpers;
module.exports = DependencyGraphHelpers;

2
packager/react-packager/src/DependencyResolver/DependencyGraph/DeprecatedAssetMap.js vendored
View File

@ -92,7 +92,7 @@ class DeprecatedAssetMap {
debug('Conflcting assets', name);
}
this._map[name] = new AssetModule_DEPRECATED(file);
this._map[name] = new AssetModule_DEPRECATED({ file });
}
}

77
packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js vendored
View File

@ -2362,6 +2362,7 @@ describe('DependencyGraph', function() {
pit('should selectively ignore providesModule in node_modules', function() {
var root = '/root';
var otherRoot = '/anotherRoot';
fs.__setMockFilesystem({
'root': {
'index.js': [
@ -2371,6 +2372,9 @@ describe('DependencyGraph', function() {
'require("shouldWork");',
'require("dontWork");',
'require("wontWork");',
'require("ember");',
'require("internalVendoredPackage");',
'require("anotherIndex");',
].join('\n'),
'node_modules': {
'react-haste': {
@ -2378,6 +2382,7 @@ describe('DependencyGraph', function() {
name: 'react-haste',
main: 'main.js',
}),
// @providesModule should not be ignored here, because react-haste is whitelisted
'main.js': [
'/**',
' * @providesModule shouldWork',
@ -2390,6 +2395,7 @@ describe('DependencyGraph', function() {
name: 'bar',
main: 'main.js',
}),
// @providesModule should be ignored here, because it's not whitelisted
'main.js':[
'/**',
' * @providesModule dontWork',
@ -2411,20 +2417,48 @@ describe('DependencyGraph', function() {
name: 'ember',
main: 'main.js',
}),
// @providesModule should be ignored here, because it's not whitelisted,
// and also, the modules "id" should be ember/main.js, not it's haste name
'main.js':[
'/**',
' * @providesModule wontWork',
' */',
'hi();',
].join('\n'),
].join('\n')
},
},
// This part of the dep graph is meant to emulate internal facebook infra.
// By whitelisting `vendored_modules`, haste should still work.
'vendored_modules': {
'a-vendored-package': {
'package.json': JSON.stringify({
name: 'a-vendored-package',
main: 'main.js',
}),
// @providesModule should _not_ be ignored here, because it's whitelisted.
'main.js':[
'/**',
' * @providesModule internalVendoredPackage',
' */',
'hiFromInternalPackage();',
].join('\n'),
}
},
},
// we need to support multiple roots and using haste between them
'anotherRoot': {
'index.js': [
'/**',
' * @providesModule anotherIndex',
' */',
'wazup()',
].join('\n'),
}
});
var dgraph = new DependencyGraph({
...defaults,
roots: [root],
roots: [root, otherRoot],
});
return getOrderedDependenciesAsJSON(dgraph, '/root/index.js').then(function(deps) {
expect(deps)
@ -2432,7 +2466,14 @@ describe('DependencyGraph', function() {
{
id: 'index',
path: '/root/index.js',
dependencies: ['shouldWork', 'dontWork', 'wontWork'],
dependencies: [
'shouldWork',
'dontWork',
'wontWork',
'ember',
'internalVendoredPackage',
'anotherIndex'
],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
@ -2459,6 +2500,36 @@ describe('DependencyGraph', function() {
isPolyfill: false,
resolution: undefined,
},
{
id: 'ember/main.js',
path: '/root/node_modules/ember/main.js',
dependencies: [],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
},
{
id: 'internalVendoredPackage',
path: '/root/vendored_modules/a-vendored-package/main.js',
dependencies: [],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
},
{
id: 'anotherIndex',
path: '/anotherRoot/index.js',
dependencies: [],
isAsset: false,
isAsset_DEPRECATED: false,
isJSON: false,
isPolyfill: false,
resolution: undefined,
},
]);
});
});

7
packager/react-packager/src/DependencyResolver/DependencyGraph/index.js vendored
View File

@ -16,7 +16,7 @@ const getPlatformExtension = require('../lib/getPlatformExtension');
const isAbsolutePath = require('absolute-path');
const path = require('path');
const util = require('util');
const Helpers = require('./Helpers');
const DependencyGraphHelpers = require('./DependencyGraphHelpers');
const ResolutionRequest = require('./ResolutionRequest');
const ResolutionResponse = require('./ResolutionResponse');
const HasteMap = require('./HasteMap');
@ -57,7 +57,7 @@ class DependencyGraph {
extractRequires,
};
this._cache = this._opts.cache;
this._helpers = new Helpers(this._opts);
this._helpers = new DependencyGraphHelpers(this._opts);
this.load().catch((err) => {
// This only happens at initialization. Live errors are easier to recover from.
console.error('Error building DependencyGraph:\n', err.stack);
@ -97,7 +97,8 @@ class DependencyGraph {
this._moduleCache = new ModuleCache(
this._fastfs,
this._cache,
this._opts.extractRequires
this._opts.extractRequires,
this._helpers
);
this._hasteMap = new HasteMap({

23
packager/react-packager/src/DependencyResolver/Module.js vendored
View File

@ -15,7 +15,7 @@ const extractRequires = require('./lib/extractRequires');
class Module {
constructor(file, fastfs, moduleCache, cache, extractor) {
constructor({ file, fastfs, moduleCache, cache, extractor, depGraphHelpers }) {
if (!isAbsolutePath(file)) {
throw new Error('Expected file to be absolute path but got ' + file);
}
@ -27,17 +27,18 @@ class Module {
this._moduleCache = moduleCache;
this._cache = cache;
this._extractor = extractor;
this._depGraphHelpers = depGraphHelpers;
}
isHaste() {
return this._read().then(data => !!data.id);
return this.read().then(data => !!data.id);
}
getName() {
return this._cache.get(
this.path,
'name',
() => this._read().then(data => {
() => this.read().then(data => {
if (data.id) {
return data.id;
}
@ -66,23 +67,31 @@ class Module {
}
getDependencies() {
return this._read().then(data => data.dependencies);
return this.read().then(data => data.dependencies);
}
getAsyncDependencies() {
return this._read().then(data => data.asyncDependencies);
return this.read().then(data => data.asyncDependencies);
}
invalidate() {
this._cache.invalidate(this.path);
}
_read() {
read() {
if (!this._reading) {
this._reading = this._fastfs.readFile(this.path).then(content => {
const data = {};
// Set an id on the module if it's using @providesModule syntax
// and if it's NOT in node_modules (and not a whitelisted node_module).
// This handles the case where a project may have a dep that has @providesModule
// docblock comments, but doesn't want it to conflict with whitelisted @providesModule
// modules, such as react-haste, fbjs-haste, or react-native or with non-dependency,
// project-specific code that is using @providesModule.
const moduleDocBlock = docblock.parseAsObject(content);
if (moduleDocBlock.providesModule || moduleDocBlock.provides) {
if (!this._depGraphHelpers.isNodeModulesDir(this.path) &&
(moduleDocBlock.providesModule || moduleDocBlock.provides)) {
data.id = /^(\S*)/.exec(
moduleDocBlock.providesModule || moduleDocBlock.provides
)[1];

40
packager/react-packager/src/DependencyResolver/ModuleCache.js vendored
View File

@ -7,25 +7,27 @@ const path = require('path');
class ModuleCache {
constructor(fastfs, cache, extractRequires) {
constructor(fastfs, cache, extractRequires, depGraphHelpers) {
this._moduleCache = Object.create(null);
this._packageCache = Object.create(null);
this._fastfs = fastfs;
this._cache = cache;
this._extractRequires = extractRequires;
this._depGraphHelpers = depGraphHelpers;
fastfs.on('change', this._processFileChange.bind(this));
}
getModule(filePath) {
filePath = path.resolve(filePath);
if (!this._moduleCache[filePath]) {
this._moduleCache[filePath] = new Module(
filePath,
this._fastfs,
this,
this._cache,
this._extractRequires
);
this._moduleCache[filePath] = new Module({
file: filePath,
fastfs: this._fastfs,
moduleCache: this,
cache: this._cache,
extractor: this._extractRequires,
depGraphHelpers: this._depGraphHelpers
});
}
return this._moduleCache[filePath];
}
@ -33,12 +35,12 @@ class ModuleCache {
getAssetModule(filePath) {
filePath = path.resolve(filePath);
if (!this._moduleCache[filePath]) {
this._moduleCache[filePath] = new AssetModule(
filePath,
this._fastfs,
this,
this._cache,
);
this._moduleCache[filePath] = new AssetModule({
file: filePath,
fastfs: this._fastfs,
moduleCache: this,
cache: this._cache,
});
}
return this._moduleCache[filePath];
}
@ -46,11 +48,11 @@ class ModuleCache {
getPackage(filePath) {
filePath = path.resolve(filePath);
if (!this._packageCache[filePath]) {
this._packageCache[filePath] = new Package(
filePath,
this._fastfs,
this._cache,
);
this._packageCache[filePath] = new Package({
file: filePath,
fastfs: this._fastfs,
cache: this._cache,
});
}
return this._packageCache[filePath];
}

12
packager/react-packager/src/DependencyResolver/Package.js vendored
View File

@ -5,7 +5,7 @@ const path = require('path');
class Package {
constructor(file, fastfs, cache) {
constructor({ file, fastfs, cache }) {
this.path = path.resolve(file);
this.root = path.dirname(this.path);
this._fastfs = fastfs;
@ -14,7 +14,7 @@ class Package {
}
getMain() {
return this._read().then(json => {
return this.read().then(json => {
var replacements = getReplacements(json);
if (typeof replacements === 'string') {
return path.join(this.root, replacements);
@ -36,13 +36,13 @@ class Package {
isHaste() {
return this._cache.get(this.path, 'package-haste', () =>
this._read().then(json => !!json.name)
this.read().then(json => !!json.name)
);
}
getName() {
return this._cache.get(this.path, 'package-name', () =>
this._read().then(json => json.name)
this.read().then(json => json.name)
);
}
@ -51,7 +51,7 @@ class Package {
}
redirectRequire(name) {
return this._read().then(json => {
return this.read().then(json => {
var replacements = getReplacements(json);
if (!replacements || typeof replacements !== 'object') {
@ -81,7 +81,7 @@ class Package {
});
}
_read() {
read() {
if (!this._reading) {
this._reading = this._fastfs.readFile(this.path)
.then(jsonStr => JSON.parse(jsonStr));

2
packager/react-packager/src/DependencyResolver/Polyfill.js vendored
View File

@ -5,7 +5,7 @@ const Module = require('./Module');
class Polyfill extends Module {
constructor({ path, id, dependencies }) {
super(path);
super({ file: path });
this._id = id;
this._dependencies = dependencies;
}

23
packager/react-packager/src/DependencyResolver/__tests__/Module-test.js vendored
View File

@ -22,6 +22,7 @@ jest
const Fastfs = require('../fastfs');
const Module = require('../Module');
const ModuleCache = require('../ModuleCache');
const DependencyGraphHelpers = require('../DependencyGraph/DependencyGraphHelpers');
const Promise = require('promise');
const fs = require('fs');
@ -50,12 +51,13 @@ describe('Module', () => {
const cache = new Cache();
return fastfs.build().then(() => {
const module = new Module(
'/root/index.js',
const module = new Module({
file: '/root/index.js',
fastfs,
new ModuleCache(fastfs, cache),
cache
);
moduleCache: new ModuleCache(fastfs, cache),
cache: cache,
depGraphHelpers: new DependencyGraphHelpers()
});
return module.getAsyncDependencies().then(actual =>
expect(actual).toEqual(expected)
@ -122,13 +124,14 @@ describe('Module', () => {
const cache = new Cache();
return fastfs.build().then(() => {
return new Module(
'/root/index.js',
return new Module({
file: '/root/index.js',
fastfs,
new ModuleCache(fastfs, cache),
moduleCache: new ModuleCache(fastfs, cache),
cache,
extractor
);
extractor,
depGraphHelpers: new DependencyGraphHelpers()
});
});
}

2
packager/react-packager/src/Resolver/__tests__/Resolver-test.js vendored
View File

@ -51,7 +51,7 @@ describe('Resolver', function() {
}
function createModule(id, dependencies) {
var module = new Module();
var module = new Module({});
module.getName.mockImpl(() => Promise.resolve(id));
module.getDependencies.mockImpl(() => Promise.resolve(dependencies));
return module;