Skip to content

Commit

Permalink
Use custom require hook to strip Flow types from NativeModule spec pa…
Browse files Browse the repository at this point in the history
…rser

Summary:
## Context
We currently run ESLint using `flow-node`. This is a very recent change that was introduced in these two diffs:
- Switch VSCode ESLint plugin using flow-node: D24454702.
- Switch all ESLint scripts to use flow-node: D24379783 (facebook@ad5802c).

## Problem
Because `react-native/eslint-plugin-codegen` (written in vanilla JavaScript) requires two files from `react-native-codegen` (written with Flow typings), we force all requires executed while initializing ESLint to compile out Flow types. Issues:
- In the grand scheme of things, this is such a tiny and isolated problem. It shouldn't be the reason why we switch over to using flow node. That's a larger decision that should be discussed outside of this diff.
- On VSCode cold start, in D24454702, I measured that using flow-node adds approximately 320ms to JavaScript file lint time. So, this is just slow.

## Solution
- Switch ESLint back to using regular node:
   - Revert the changes to VSCode's ESLint plugin: D24454702
   - Revert the changes to the internal ESLint scripts: D24379783 (facebook@ad5802c).
- Inside the ESLint plugin, register a temporary require hook to remove Flow types from the NativeModule spec parser, before we require it. We de-register this hook after the requires finish.

## Implementation Notes:
- The `with-babel-register/` is a fork of `babel/register`, except I simplified the implementation based on the assumption that we're using it literally to only compile `react-native-codegen`.
- `with-babel-register/` uses a disk cache, so we only call transformSync when a the input file (1) hasn't been transformed before, or (2) the cache entry was created before the file was last modified.
- I ported over the source-map logic, so that when the NativeModule spec parser throws non-parsing errors, we get the correct stack trace. **Note:** I'm not sure if the source maps will work if there's a babel/register earlier during initialization. However, I don't think this will pose an actual problem, since we don't use a babel/register hook earlier. So, I think we should punt on this investigation.

## Alternative: Why aren't we using babel/register?
Every time you call babel/register, it replaces the last registered hook. We don't want the ESLint plugin to be changing any existing require hooks that people have set up. Abandoned diff with babel/register: D24519349.

Changelog: [Internal]

Reviewed By: cpojer

Differential Revision: D24551549

fbshipit-source-id: bbd7c5be44f74c0e9adbb20fe86e09802410b123
  • Loading branch information
RSNara authored and facebook-github-bot committed Oct 27, 2020
1 parent f023519 commit 758b633
Show file tree
Hide file tree
Showing 5 changed files with 282 additions and 19 deletions.
7 changes: 6 additions & 1 deletion packages/eslint-plugin-codegen/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@
"directory": "packages/eslint-plugin-codegen"
},
"dependencies": {
"@babel/core": "^7.0.0",
"@babel/plugin-transform-flow-strip-types": "^7.0.0",
"flow-parser": "^0.121.0",
"react-native-codegen": "*"
"make-dir": "^2.1.0",
"pirates":"^4.0.1",
"react-native-codegen": "*",
"source-map-support": "0.5.0"
},
"license": "MIT"
}
46 changes: 28 additions & 18 deletions packages/eslint-plugin-codegen/react-native-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'use strict';

const path = require('path');
const withBabelRegister = require('./with-babel-register');

const ERRORS = {
misnamedHasteModule(hasteModuleName) {
Expand Down Expand Up @@ -38,6 +39,28 @@ const ERRORS = {
},
};

let RNModuleParser;
let RNParserUtils;

function requireModuleParser() {
if (RNModuleParser == null || RNParserUtils == null) {
const config = {
only: [/react-native-codegen\/src\//],
plugins: [require('@babel/plugin-transform-flow-strip-types').default],
};

withBabelRegister(config, () => {
RNModuleParser = require('react-native-codegen/src/parsers/flow/modules');
RNParserUtils = require('react-native-codegen/src/parsers/flow/utils');
});
}

return {
buildModuleSchema: RNModuleParser.buildModuleSchema,
createParserErrorCapturer: RNParserUtils.createParserErrorCapturer,
};
}

const VALID_SPEC_NAMES = /^Native\S+$/;

function isModuleRequire(node) {
Expand Down Expand Up @@ -121,25 +144,12 @@ function rule(context) {
});
}

let buildModuleSchema = null;
let createParserErrorCapturer = null;
try {
/**
* The following files are written with Flow typings.
* Unless the typings are stripped at compile-time or run-time,
* the following requires fill fail. Hence, the try/catch.
*/
({
buildModuleSchema,
} = require('react-native-codegen/src/parsers/flow/modules'));
({
createParserErrorCapturer,
} = require('react-native-codegen/src/parsers/flow/utils'));
} catch (ex) {
return;
}

const {
buildModuleSchema,
createParserErrorCapturer,
} = requireModuleParser();
const flowParser = require('flow-parser');

const [parsingErrors, guard] = createParserErrorCapturer();

const sourceCode = context.getSourceCode().getText();
Expand Down
131 changes: 131 additions & 0 deletions packages/eslint-plugin-codegen/with-babel-register/disk-cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react_native
* @format
*/

const path = require('path');
const fs = require('fs');
const os = require('os');
const {sync: makeDirSync} = require('make-dir');

const packageJson = JSON.parse(
fs.readFileSync(require.resolve('../package.json'), 'utf8'),
);

/**
* This file is a fork of
* https://github.com/babel/babel/blob/2782a549e99d2ef1816332d23d7dfd5190f58a0f/packages/babel-register/src/cache.js#L1
*/

const FILENAME = path.join(
os.tmpdir(),
`.eslint-plugin-codegen.${packageJson.version}.disk-cache.json`,
);

let data = {};

let cacheDisabled = process.env.NODE_ENV === 'test';

function isCacheDisabled() {
return cacheDisabled;
}

/**
* Write stringified cache to disk.
*/
function save() {
if (isCacheDisabled()) {
return;
}

let serialised = '{}';

try {
serialised = JSON.stringify(data, null, ' ');
} catch (err) {
if (err.message === 'Invalid string length') {
err.message = "Cache too large so it's been cleared.";
console.error(err.stack);
} else {
throw err;
}
}

try {
makeDirSync(path.dirname(FILENAME));
fs.writeFileSync(FILENAME, serialised);
} catch (e) {
switch (e.code) {
// workaround https://github.com/nodejs/node/issues/31481
// todo: remove the ENOENT error check when we drop node.js 13 support
case 'ENOENT':
case 'EACCES':
case 'EPERM':
console.warn(
`Could not write cache to file: ${FILENAME} due to a permission issue. Cache is disabled.`,
);
cacheDisabled = true;
break;
case 'EROFS':
console.warn(
`Could not write cache to file: ${FILENAME} because it resides in a readonly filesystem. Cache is disabled.`,
);
cacheDisabled = true;
break;
default:
throw e;
}
}
}

/**
* Load cache from disk and parse.
*/

function load() {
if (isCacheDisabled()) {
data = {};
return;
}

process.on('exit', save);
process.nextTick(save);

let cacheContent;

try {
cacheContent = fs.readFileSync(FILENAME);
} catch (e) {
switch (e.code) {
// check EACCES only as fs.readFileSync will never throw EPERM on Windows
// https://github.com/libuv/libuv/blob/076df64dbbda4320f93375913a728efc40e12d37/src/win/fs.c#L735
case 'EACCES':
console.warn(
`Babel could not read cache file: ${FILENAME} due to a permission issue. Cache is disabled.`,
);
cacheDisabled = true;
/* fall through */
default:
return;
}
}

try {
data = JSON.parse(cacheContent);
} catch {}
}

/**
* Retrieve data from cache.
*/

function get() {
return data;
}

module.exports = {load, get};
110 changes: 110 additions & 0 deletions packages/eslint-plugin-codegen/with-babel-register/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react_native
* @format
*/

const babel = require('@babel/core');
const {OptionManager, DEFAULT_EXTENSIONS} = require('@babel/core');
const sourceMapSupport = require('source-map-support');
const {addHook} = require('pirates');
const path = require('path');
const fs = require('fs');
const diskCache = require('./disk-cache');

function compile(sourceMapManager, cache, options, code, filename) {
const opts = new OptionManager().init({
sourceRoot: path.dirname(filename) + path.sep,
...options,
filename,
});

// Bail out ASAP if the file has been ignored.
if (opts === null) {
return code;
}

let output = cache[filename];

if (!output || output.mtime !== mtime(filename)) {
output = babel.transformSync(code, {
...opts,
sourceMaps: opts.sourceMaps === undefined ? 'both' : opts.sourceMaps,
ast: false,
});

cache[filename] = output;
output.mtime = mtime(filename);
}

if (!sourceMapManager.isInstalled) {
sourceMapManager.install();
}

if (output.map) {
sourceMapManager.maps[filename] = output.map;
}

return output.code;
}

function mtime(filename) {
return +fs.statSync(filename).mtime;
}

function withBabelRegister(options, fn) {
let revertHook;
/**
* TODO: Do source maps break when we use a require hook
* to before we initialize the ESLint plugin?
*/
const sourceMapManager = {
isInstalled: false,
maps: {},
install() {
if (sourceMapManager.isInstalled) {
return;
}
sourceMapManager.isInstalled = true;
sourceMapSupport.install({
handleUncaughtExceptions: true,
environment: 'node',
retrieveSourceMap(filename) {
const map = sourceMapManager.maps && sourceMapManager.maps[filename];
if (map) {
return {
url: null,
map: map,
};
} else {
return null;
}
},
});
},
};

diskCache.load();
const cache = diskCache.get();

try {
revertHook = addHook(
(code, filename) => {
return compile(sourceMapManager, cache, options, code, filename);
},
{
exts: DEFAULT_EXTENSIONS,
ignoreNodeModules: false,
},
);
return fn();
} finally {
revertHook();
}
}

module.exports = withBabelRegister;
7 changes: 7 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6480,6 +6480,13 @@ source-map-resolve@^0.5.0:
source-map-url "^0.4.0"
urix "^0.1.0"

source-map-support@0.5.0:
version "0.5.0"
resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.0.tgz#2018a7ad2bdf8faf2691e5fddab26bed5a2bacab"
integrity sha512-vUoN3I7fHQe0R/SJLKRdKYuEdRGogsviXFkHHo17AWaTGv17VLnxw+CFXvqy+y4ORZ3doWLQcxRYfwKrsd/H7Q==
dependencies:
source-map "^0.6.0"

source-map-support@^0.5.16, source-map-support@^0.5.6:
version "0.5.16"
resolved "https://registry.yarnpkg.com/source-map-support/-/source-map-support-0.5.16.tgz#0ae069e7fe3ba7538c64c98515e35339eac5a042"
Expand Down

0 comments on commit 758b633

Please sign in to comment.