Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(build): add basic inline-fs plugin #13232

Merged
merged 2 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
218 changes: 218 additions & 0 deletions build/plugins/inline-fs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
/**
* @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

const assert = require('assert').strict;
const fs = require('fs');
const acorn = require('acorn');
const MagicString = require('magic-string').default;

// ESTree provides much better types for AST nodes. See https://github.com/acornjs/acorn/issues/946
/** @typedef {import('estree').Node} Node */
/** @typedef {import('estree').SimpleCallExpression} SimpleCallExpression */

/**
* Inlines the values of selected `fs` methods if their targets can be
* statically determined. Currently `readFileSync` and `readdirSync` are
* supported.
* Returns `null` as code if no changes were made.
* @param {string} code
* @return {Promise<string|null>}
*/
async function inlineFs(code) {
// Approach:
// - scan `code` for fs methods
// - parse only the expression at each found index
// - statically evaluate arguments to fs method, collapsing to single string
// - execute fs method with computed argument
// - replace original expression with result of fs call
// - if an expression cannot be parsed or statically evaluated, warn and skip
// - if no expressions found or all are skipped, return null

const fsSearch = /fs\.(?:readFileSync|readdirSync)\(/g;
const foundIndices = [...code.matchAll(fsSearch)].map(e => e.index);

// Return null for not-applicable files with as little work as possible.
if (foundIndices.length === 0) return null;

const output = new MagicString(code);
let madeChange = false;

// Can iterate forwards in string because MagicString always uses original indices.
for (const foundIndex of foundIndices) {
if (foundIndex === undefined) continue; // https://github.com/microsoft/TypeScript/issues/36788

let parsed;
try {
parsed = parseExpressionAt(code, foundIndex, {ecmaVersion: 'latest'});
} catch (err) {
console.warn(err.message);
continue;
}

// If root of expression isn't the fs call, descend down chained methods on
// the result (e.g. `fs.readdirSync().map(...)`) until reaching the fs call.
for (;;) {
assertEqualString(parsed.type, 'CallExpression');
assertEqualString(parsed.callee.type, 'MemberExpression');
if (parsed.callee.object.type === 'Identifier' && parsed.callee.object.name === 'fs') {
break;
}
parsed = parsed.callee.object;
}

// We've regexed for an fs method, so the property better be an identifier.
assertEqualString(parsed.callee.property.type, 'Identifier');

let content;
try {
if (parsed.callee.property.name === 'readFileSync') {
content = await getReadFileReplacement(parsed);
} else if (parsed.callee.property.name === 'readdirSync') {
content = await getReaddirReplacement(parsed);
} else {
throw new Error(`unexpected fs call 'fs.${parsed.callee.property.name}'`);
}
} catch (err) {
console.warn(err.message);
continue;
}

// @ts-expect-error - `start` and `end` provided by acorn on top of ESTree types.
const {start, end} = parsed;
// TODO(bckenny): use options to customize `storeName` for source maps.
output.overwrite(start, end, content);
madeChange = true;
}

// Be explicit if no change has been made.
const outputCode = madeChange ? output.toString() : null;

return outputCode;
}

/**
* A version of acorn's parseExpressionAt that stops at commas, allowing parsing
* non-sequence expressions (like inside arrays).
* @param {string} input
* @param {number} offset
* @param {import('acorn').Options} options
* @return {Node}
*/
function parseExpressionAt(input, offset, options) {
const parser = new acorn.Parser(options, input, offset);
// @ts-expect-error - Not part of the current acorn types.
parser.nextToken();
// @ts-expect-error - Not part of the current acorn types.
Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, these aren't part of the current acorn.d.ts, but between the shipped types and using the estree Node type we don't have to maintain our own type file, which seems like a big win

return parser.parseMaybeAssign();
}

/**
* Uses assert.strictEqual, but does not widen the type to generic `string`,
* preserving string literals (if applicable).
* @template {string} T
* @param {string} actual
* @param {T} expected
* @param {string=} errorMessage
* @return {asserts actual is T}
*/
function assertEqualString(actual, expected, errorMessage) {
assert.equal(actual, expected, errorMessage);
}

/**
* Attempts to statically determine the target of a `fs.readFileSync()` call and
* returns the already-quoted contents of the file to be loaded.
* If it's a JS file, it's minified before inlining.
* @param {SimpleCallExpression} node ESTree node for `fs.readFileSync` call.
* @return {Promise<string>}
*/
async function getReadFileReplacement(node) {
assertEqualString(node.callee.type, 'MemberExpression');
assertEqualString(node.callee.property.type, 'Identifier');
assert.equal(node.callee.property.name, 'readFileSync');

assert.equal(node.arguments.length, 2, 'fs.readFileSync() must have two arguments');
const constructedPath = collapseToStringLiteral(node.arguments[0]);
assert.equal(isUtf8Options(node.arguments[1]), true, 'only utf8 readFileSync is supported');

const readContent = await fs.promises.readFile(constructedPath, 'utf8');

// TODO(bckenny): minify inlined javascript.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this part necessary? I think the subsequent steps of the bundler will handle the minification.

Copy link
Member Author

Choose a reason for hiding this comment

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

this is for the inlined string, which the minification at the bundler level will ignore as just a string literal. We're manually running the current minifyFileTransform rather than passing it in, more or less

Copy link
Collaborator

Choose a reason for hiding this comment

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

doh! of course. sgtm


// Escape quotes, new lines, etc so inlined string doesn't break host file.
return JSON.stringify(readContent);
}

/**
* Attempts to statically determine the target of a `fs.readdirSync()` call and
* returns a JSON.stringified array with the contents of the target directory.
* @param {SimpleCallExpression} node ESTree node for `fs.readdirSync` call.
* @return {Promise<string>}
*/
async function getReaddirReplacement(node) {
assertEqualString(node.callee.type, 'MemberExpression');
assertEqualString(node.callee.property.type, 'Identifier');
assert.equal(node.callee.property.name, 'readdirSync');

// If there's no second argument, fs.readdirSync defaults to 'utf8'.
if (node.arguments.length === 2) {
assert.equal(isUtf8Options(node.arguments[1]), true, 'only utf8 readdirSync is supported');
}

const constructedPath = collapseToStringLiteral(node.arguments[0]);

try {
const contents = await fs.promises.readdir(constructedPath, 'utf8');
return JSON.stringify(contents);
} catch (err) {
throw new Error(`could not inline fs.readdirSync call: ${err.message}`);
}
}

/**
* Returns whether the options object/string specifies the allowed utf8/utf-8
* encoding.
* @param {Node} node ESTree node.
* @return {boolean}
*/
function isUtf8Options(node) {
// Node allows 'utf-8' as an alias for 'utf8'.
if (node.type === 'Literal') {
return node.value === 'utf8' || node.value === 'utf-8';
} else if (node.type === 'ObjectExpression') {
// Matches type `{encoding: 'utf8'|'utf-8'}`.
return node.properties.some(prop => {
return prop.type === 'Property' &&
prop.key.type === 'Identifier' && prop.key.name === 'encoding' &&
prop.value.type === 'Literal' &&
(prop.value.value === 'utf8' || prop.value.value === 'utf-8');
});
}
return false;
}

/**
* Collapse tree at `node` using supported transforms until only a string
* literal is returned (or an error is thrown for unsupported nodes).
* @param {Node} node ESTree node.
* @return {string}
*/
function collapseToStringLiteral(node) {
// TODO(bckenny): support more than string literals.
switch (node.type) {
case 'Literal': {
// If your literal wasn't a string, sorry, you're getting a string.
return String(node.value);
}
}

throw new Error(`unsupported node: ${node.type}`);
}

module.exports = {
inlineFs,
};
152 changes: 152 additions & 0 deletions build/test/plugins/inline-fs-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/**
* @license Copyright 2021 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/* eslint-env jest */

const fs = require('fs');
const path = require('path');
const {inlineFs} = require('../../plugins/inline-fs.js');

const {LH_ROOT} = require('../../../root.js');

describe('inline-fs', () => {
const tmpPath = `${LH_ROOT}/.tmp/inline-fs/test.txt`;
const tmpDir = path.dirname(tmpPath);

beforeAll(() => {
fs.mkdirSync(tmpDir, {recursive: true});
});

afterAll(() => {
fs.unlinkSync(tmpPath);
});

describe('supported syntax', () => {
it('returns null for content with no fs calls', async () => {
const content = 'const val = 1;';
const result = await inlineFs(content);
expect(result).toEqual(null);
});

it('returns null for non-call references to fs methods', async () => {
const content = 'const val = fs.readFileSync ? 1 : 2;';
const result = await inlineFs(content);
expect(result).toEqual(null);
});

it('evaluates an fs.readFileSync call and inlines the contents', async () => {
fs.writeFileSync(tmpPath, 'template literal text content');

const content = `const myTextContent = fs.readFileSync('${tmpPath}', 'utf8');`;
const code = await inlineFs(content);
expect(code).toBe(`const myTextContent = "template literal text content";`);
});

it('warns and skips unsupported construct but inlines subsequent fs method calls', async () => {
fs.writeFileSync(tmpPath, 'template literal text content');

// eslint-disable-next-line max-len
const content = `const myContent = fs.readFileSync(filePathVar, 'utf8');\nconst replacedContent = fs.readFileSync('${tmpPath}', 'utf8');`;
const code = await inlineFs(content);
// eslint-disable-next-line max-len
expect(code).toBe(`const myContent = fs.readFileSync(filePathVar, 'utf8');\nconst replacedContent = "template literal text content";`);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

add a TODO to check for a warning (for once you've added that)


// TODO: this will work
Copy link
Collaborator

Choose a reason for hiding this comment

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

meant to leave this comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

meant to leave this comment?

yeah, just a note that it's not able to handle it now but will replace __dirname when the full thing is implemented

it('skips `__dirname`', async () => {
fs.writeFileSync(tmpPath, '__dirname text content');

const dirnamePath = `__dirname + '/../.tmp/inline-fs/test.txt'`;
const content = `const myTextContent = fs.readFileSync(${dirnamePath}, 'utf8');`;
const code = await inlineFs(content);
expect(code).toBe(null);
});

describe('fs.readFileSync', () => {
it('inlines content with quotes', async () => {
fs.writeFileSync(tmpPath, `"quoted", and an unbalanced quote: "`);
const content = `const myTextContent = fs.readFileSync('${tmpPath}', 'utf8');`;
const code = await inlineFs(content);
expect(code).toBe(`const myTextContent = "\\"quoted\\", and an unbalanced quote: \\"";`);
});

it('inlines multiple fs.readFileSync calls', async () => {
fs.writeFileSync(tmpPath, 'some text content');
// const content = `fs.readFileSync('${tmpPath}', 'utf8')fs.readFileSync(require.resolve('${tmpPath}'), 'utf8')`;
// eslint-disable-next-line max-len
const content = `fs.readFileSync('${tmpPath}', 'utf8')fs.readFileSync('${tmpPath}', 'utf8')`;
const code = await inlineFs(content);
expect(code).toBe(`"some text content""some text content"`);
});

it('inlines content from fs.readFileSync with variants of utf8 options', async () => {
fs.writeFileSync(tmpPath, 'some text content');

const utf8Variants = [
`'utf8'`,
`'utf-8'`,
`{encoding: 'utf8'}`,
`{encoding: 'utf-8'}`,
`{encoding: 'utf8', nonsense: 'flag'}`,
];

for (const opts of utf8Variants) {
const content = `const myTextContent = fs.readFileSync('${tmpPath}', ${opts});`;
const code = await inlineFs(content);
expect(code).toBe(`const myTextContent = "some text content";`);
}
});
});

describe('fs.readdirSync', () => {
it('inlines content from fs.readdirSync calls', async () => {
fs.writeFileSync(tmpPath, 'text');
const content = `const files = fs.readdirSync('${tmpDir}');`;
const code = await inlineFs(content);
const tmpFilename = path.basename(tmpPath);
expect(code).toBe(`const files = ["${tmpFilename}"];`);
});

it('handles methods chained on fs.readdirSync result', async () => {
fs.writeFileSync(tmpPath, 'text');
// eslint-disable-next-line max-len
const content = `const files = [...fs.readdirSync('${tmpDir}'), ...fs.readdirSync('${tmpDir}').map(f => \`metrics/\${f}\`)]`;
const code = await inlineFs(content);
// eslint-disable-next-line max-len
expect(code).toBe('const files = [...["test.txt"], ...["test.txt"].map(f => `metrics/${f}`)]');
});

it('inlines content from fs.readdirSync with variants of utf8 options', async () => {
fs.writeFileSync(tmpPath, 'text');
const tmpFilename = path.basename(tmpPath);

const utf8Variants = [
'', // `options` are optional for readdirSync, so include missing opts.
`'utf8'`,
`'utf-8'`,
`{encoding: 'utf8'}`,
`{encoding: 'utf-8'}`,
`{encoding: 'utf8', nonsense: 'flag'}`,
];

for (const opts of utf8Variants) {
// Trailing comma has no effect in missing opts case.
const content = `const files = fs.readdirSync('${tmpDir}', ${opts});`;
const code = await inlineFs(content);
expect(code).toBe(`const files = ["${tmpFilename}"];`);
}
});

it('throws when trying to fs.readdirSync a non-existent directory', async () => {
const nonsenseDir = `${LH_ROOT}/.tmp/nonsense-path/`;
const content = `const files = fs.readdirSync('${nonsenseDir}');`;
const code = await inlineFs(content);
expect(code).toBe(null);
});
});
});
});
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module.exports = {
'**/third-party/**/*-test.js',
'**/clients/test/**/*-test.js',
'**/shared/**/*-test.js',
'**/build/**/*-test.js',
],
transform: {},
prettierPath: null,
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/c8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ set -euxo pipefail
echo $*

node node_modules/.bin/c8 \
--include '{lighthouse-core,lighthouse-cli,lighthouse-viewer,lighthouse-treemap}' \
--include '{lighthouse-core,lighthouse-cli,lighthouse-viewer,lighthouse-treemap,build/plugins}' \
--exclude third_party \
--exclude '**/test/' \
--exclude '**/scripts/' \
Expand Down
Loading