From 61b3129808b27ca0821e9fc0286b893e6fdc2898 Mon Sep 17 00:00:00 2001 From: kzc Date: Sat, 4 Jan 2020 14:37:25 -0500 Subject: [PATCH] Implement stdin input with optional "-" as the file name. (#3290) * Implement stdin input with optional "-" as the file name. Set default output format to "es" to save CLI typing for the most common case. closes #1440 closes #3276 * * Test if we can delete process.stdin on CI * Only allow '-' to use stdin with the JS API * Use locally installed shx * Add test for stdin read error Co-authored-by: Lukas Taegert-Atkinson --- cli/run/index.ts | 8 +++ src/utils/defaultPlugin.ts | 5 +- src/utils/mergeOptions.ts | 15 +++++- src/utils/stdin.ts | 54 +++++++++++++++++++ test/cli/index.js | 11 +++- .../samples/stdin-multiple-targets/_config.js | 4 ++ .../stdin-multiple-targets/_expected/cjs.js | 12 +++++ .../stdin-multiple-targets/_expected/es.js | 7 +++ test/cli/samples/stdin-multiple-targets/a.mjs | 1 + test/cli/samples/stdin-multiple-targets/b.mjs | 1 + .../stdin-multiple-targets/rollup.config.js | 16 ++++++ test/cli/samples/stdin-no-dash/_config.js | 4 ++ .../samples/stdin-no-dash/_expected/out.js | 1 + test/cli/samples/stdin-self-import/_config.js | 4 ++ .../stdin-self-import/_expected/out.js | 11 ++++ test/cli/samples/stdin-self-import/input.txt | 10 ++++ test/cli/samples/stdin-with-dash/_config.js | 4 ++ .../samples/stdin-with-dash/_expected/out.js | 1 + test/function/index.js | 4 +- .../samples/stdin-not-present/_config.js | 23 ++++++++ .../samples/stdin-not-present/main.js | 1 + .../samples/stdin-read-error/_config.js | 29 ++++++++++ .../function/samples/stdin-read-error/main.js | 1 + test/function/samples/stdin-watch/_config.js | 11 ++++ test/function/samples/stdin-watch/main.js | 1 + test/misc/sanity-checks.js | 21 +++++++- 26 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 src/utils/stdin.ts create mode 100644 test/cli/samples/stdin-multiple-targets/_config.js create mode 100644 test/cli/samples/stdin-multiple-targets/_expected/cjs.js create mode 100644 test/cli/samples/stdin-multiple-targets/_expected/es.js create mode 100644 test/cli/samples/stdin-multiple-targets/a.mjs create mode 100644 test/cli/samples/stdin-multiple-targets/b.mjs create mode 100644 test/cli/samples/stdin-multiple-targets/rollup.config.js create mode 100644 test/cli/samples/stdin-no-dash/_config.js create mode 100644 test/cli/samples/stdin-no-dash/_expected/out.js create mode 100644 test/cli/samples/stdin-self-import/_config.js create mode 100644 test/cli/samples/stdin-self-import/_expected/out.js create mode 100644 test/cli/samples/stdin-self-import/input.txt create mode 100644 test/cli/samples/stdin-with-dash/_config.js create mode 100644 test/cli/samples/stdin-with-dash/_expected/out.js create mode 100644 test/function/samples/stdin-not-present/_config.js create mode 100644 test/function/samples/stdin-not-present/main.js create mode 100644 test/function/samples/stdin-read-error/_config.js create mode 100644 test/function/samples/stdin-read-error/main.js create mode 100644 test/function/samples/stdin-watch/_config.js create mode 100644 test/function/samples/stdin-watch/main.js diff --git a/cli/run/index.ts b/cli/run/index.ts index 5bf7d73dd6e..ced44e4db0f 100644 --- a/cli/run/index.ts +++ b/cli/run/index.ts @@ -3,6 +3,7 @@ import relative from 'require-relative'; import { WarningHandler } from '../../src/rollup/types'; import mergeOptions, { GenericConfigObject } from '../../src/utils/mergeOptions'; import { getAliasName } from '../../src/utils/relativeId'; +import { stdinName } from '../../src/utils/stdin'; import { handleError } from '../logging'; import batchWarnings from './batchWarnings'; import build from './build'; @@ -101,6 +102,7 @@ function execute (configFile: string, configs: GenericConfigObject[], command: a for (const config of configs) { promise = promise.then(() => { const warnings = batchWarnings(); + handleMissingInput(command, config); const { inputOptions, outputOptions, optionError } = mergeOptions({ command, config, @@ -115,3 +117,9 @@ function execute (configFile: string, configs: GenericConfigObject[], command: a return promise; } } + +function handleMissingInput(command: any, config: GenericConfigObject) { + if (!(command.input || config.input || config.input === '' || process.stdin.isTTY)) { + command.input = stdinName; + } +} diff --git a/src/utils/defaultPlugin.ts b/src/utils/defaultPlugin.ts index 7c93d36c9de..e0ed25fecc8 100644 --- a/src/utils/defaultPlugin.ts +++ b/src/utils/defaultPlugin.ts @@ -2,13 +2,14 @@ import { Plugin, ResolveIdHook } from '../rollup/types'; import { error } from './error'; import { lstatSync, readdirSync, readFile, realpathSync } from './fs'; import { basename, dirname, isAbsolute, resolve } from './path'; +import { readStdin, stdinName } from './stdin'; export function getRollupDefaultPlugin(preserveSymlinks: boolean): Plugin { return { name: 'Rollup Core', resolveId: createResolveId(preserveSymlinks) as ResolveIdHook, load(id) { - return readFile(id); + return id === stdinName ? readStdin() : readFile(id); }, resolveFileUrl({ relativePath, format }) { return relativeUrlMechanisms[format](relativePath); @@ -58,6 +59,8 @@ function createResolveId(preserveSymlinks: boolean) { }); } + if (source === stdinName) return source; + // external modules (non-entry modules that start with neither '.' or '/') // are skipped at this stage. if (importer !== undefined && !isAbsolute(source) && source[0] !== '.') return null; diff --git a/src/utils/mergeOptions.ts b/src/utils/mergeOptions.ts index d6332a16f77..943c4eedb1f 100644 --- a/src/utils/mergeOptions.ts +++ b/src/utils/mergeOptions.ts @@ -5,6 +5,8 @@ import { WarningHandlerWithDefault } from '../rollup/types'; +import { stdinName } from './stdin'; + export interface GenericConfigObject { [key: string]: unknown; } @@ -208,6 +210,7 @@ function getInputOptions( defaultOnWarnHandler: WarningHandler ): InputOptions { const getOption = createGetOption(config, command); + const input = getOption('input', []); const inputOptions: InputOptions = { acorn: config.acorn, @@ -220,7 +223,7 @@ function getInputOptions( experimentalTopLevelAwait: getOption('experimentalTopLevelAwait'), external: getExternal(config, command) as any, inlineDynamicImports: getOption('inlineDynamicImports', false), - input: getOption('input', []), + input, manualChunks: getOption('manualChunks'), moduleContext: config.moduleContext as any, onwarn: getOnWarn(config, defaultOnWarnHandler), @@ -234,6 +237,13 @@ function getInputOptions( watch: config.watch as any }; + if ( + config.watch && + (input === stdinName || (Array.isArray(input) && input.indexOf(stdinName) >= 0)) + ) { + throw new Error('watch mode is incompatible with stdin input'); + } + // support rollup({ cache: prevBuildObject }) if (inputOptions.cache && (inputOptions.cache as any).cache) inputOptions.cache = (inputOptions.cache as any).cache; @@ -250,6 +260,7 @@ function getOutputOptions( // Handle format aliases switch (format) { + case undefined: case 'esm': case 'module': format = 'es'; @@ -273,7 +284,7 @@ function getOutputOptions( externalLiveBindings: getOption('externalLiveBindings', true), file: getOption('file'), footer: getOption('footer'), - format: format === 'esm' ? 'es' : format, + format, freeze: getOption('freeze', true), globals: getOption('globals'), indent: getOption('indent', true), diff --git a/src/utils/stdin.ts b/src/utils/stdin.ts new file mode 100644 index 00000000000..d2aa4d73005 --- /dev/null +++ b/src/utils/stdin.ts @@ -0,0 +1,54 @@ +export const stdinName = '-'; + +let stdinResult: string | Error | null = null; +const pending: { reject: Function; resolve: Function }[] = []; + +export function readStdin() { + return new Promise((resolve, reject) => { + if (typeof process == 'undefined' || typeof process.stdin == 'undefined') { + reject(new Error('stdin input is invalid in browser')); + return; + } + pending.push({ resolve, reject }); + processPending(); + if (pending.length === 1) { + // stdin is read once - all callers will get the same result + + const chunks: Buffer[] = []; + process.stdin.setEncoding('utf8'); + process.stdin + .on('data', chunk => { + if (stdinResult === null) { + chunks.push(chunk); + } + }) + .on('end', () => { + if (stdinResult === null) { + stdinResult = chunks.join(''); + chunks.length = 0; + } + processPending(); + }) + .on('error', err => { + if (stdinResult === null) { + stdinResult = err instanceof Error ? err : new Error(err); + chunks.length = 0; + } + processPending(); + }); + process.stdin.resume(); + } + }); + + function processPending() { + if (stdinResult !== null) { + for (let it; (it = pending.shift()); ) { + if (typeof stdinResult == 'string') { + it.resolve(stdinResult); + } else { + it.reject(stdinResult); + } + } + } + } +} diff --git a/test/cli/index.js b/test/cli/index.js index 3bef6e75a37..10277dbc2df 100644 --- a/test/cli/index.js +++ b/test/cli/index.js @@ -23,8 +23,15 @@ runTestSuiteWithSamples( done => { process.chdir(config.cwd || dir); - const command = - 'node ' + path.resolve(__dirname, '../../dist/bin') + path.sep + config.command; + const command = config.command + .replace( + /(^| )rollup /g, + `node ${path.resolve(__dirname, '../../dist/bin')}${path.sep}rollup ` + ) + .replace( + /(^| )shx /g, + `node ${path.resolve(__dirname, '../../node_modules/.bin')}${path.sep}shx ` + ); const childProcess = exec( command, diff --git a/test/cli/samples/stdin-multiple-targets/_config.js b/test/cli/samples/stdin-multiple-targets/_config.js new file mode 100644 index 00000000000..4e4132f286d --- /dev/null +++ b/test/cli/samples/stdin-multiple-targets/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: 'uses stdin in multiple targets', + command: `shx echo "import {PRINT as p} from './a'; import C from './b'; 0 && fail() || p(C); export {C as value, p as print}" | rollup -c` +}; diff --git a/test/cli/samples/stdin-multiple-targets/_expected/cjs.js b/test/cli/samples/stdin-multiple-targets/_expected/cjs.js new file mode 100644 index 00000000000..ece324c0d9e --- /dev/null +++ b/test/cli/samples/stdin-multiple-targets/_expected/cjs.js @@ -0,0 +1,12 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); + +const PRINT = x => console.log(x); + +var C = 123; + +PRINT(C); + +exports.print = PRINT; +exports.value = C; diff --git a/test/cli/samples/stdin-multiple-targets/_expected/es.js b/test/cli/samples/stdin-multiple-targets/_expected/es.js new file mode 100644 index 00000000000..0535634ff8f --- /dev/null +++ b/test/cli/samples/stdin-multiple-targets/_expected/es.js @@ -0,0 +1,7 @@ +const PRINT = x => console.log(x); + +var C = 123; + +PRINT(C); + +export { PRINT as print, C as value }; diff --git a/test/cli/samples/stdin-multiple-targets/a.mjs b/test/cli/samples/stdin-multiple-targets/a.mjs new file mode 100644 index 00000000000..f98de3b69af --- /dev/null +++ b/test/cli/samples/stdin-multiple-targets/a.mjs @@ -0,0 +1 @@ +export const PRINT = x => console.log(x); diff --git a/test/cli/samples/stdin-multiple-targets/b.mjs b/test/cli/samples/stdin-multiple-targets/b.mjs new file mode 100644 index 00000000000..05e08712040 --- /dev/null +++ b/test/cli/samples/stdin-multiple-targets/b.mjs @@ -0,0 +1 @@ +export default 123; diff --git a/test/cli/samples/stdin-multiple-targets/rollup.config.js b/test/cli/samples/stdin-multiple-targets/rollup.config.js new file mode 100644 index 00000000000..5cc78cddbbf --- /dev/null +++ b/test/cli/samples/stdin-multiple-targets/rollup.config.js @@ -0,0 +1,16 @@ +export default [ + { + input: '-', + output: { + file: '_actual/cjs.js', + format: 'cjs' + } + }, + { + input: '-', + output: { + file: '_actual/es.js', + format: 'es' + } + } +]; diff --git a/test/cli/samples/stdin-no-dash/_config.js b/test/cli/samples/stdin-no-dash/_config.js new file mode 100644 index 00000000000..33705242ff6 --- /dev/null +++ b/test/cli/samples/stdin-no-dash/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: 'stdin input with no dash on CLI', + command: `shx mkdir -p _actual && shx echo "0 && fail() || console.log('PASS');" | rollup > _actual/out.js` +}; diff --git a/test/cli/samples/stdin-no-dash/_expected/out.js b/test/cli/samples/stdin-no-dash/_expected/out.js new file mode 100644 index 00000000000..80d7f8cb462 --- /dev/null +++ b/test/cli/samples/stdin-no-dash/_expected/out.js @@ -0,0 +1 @@ +console.log('PASS'); diff --git a/test/cli/samples/stdin-self-import/_config.js b/test/cli/samples/stdin-self-import/_config.js new file mode 100644 index 00000000000..691426d2b24 --- /dev/null +++ b/test/cli/samples/stdin-self-import/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: 'stdin input of code that imports a copy of itself', + command: `shx mkdir -p _actual && shx cat input.txt | rollup -f cjs --silent > _actual/out.js` +}; diff --git a/test/cli/samples/stdin-self-import/_expected/out.js b/test/cli/samples/stdin-self-import/_expected/out.js new file mode 100644 index 00000000000..00ecc7931f4 --- /dev/null +++ b/test/cli/samples/stdin-self-import/_expected/out.js @@ -0,0 +1,11 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); + +let b = 2; +var a = 4; + +console.log(a, b); + +exports.b = b; +exports.default = a; diff --git a/test/cli/samples/stdin-self-import/input.txt b/test/cli/samples/stdin-self-import/input.txt new file mode 100644 index 00000000000..0f74081903a --- /dev/null +++ b/test/cli/samples/stdin-self-import/input.txt @@ -0,0 +1,10 @@ +import a from "-"; +import { b as c } from "-"; + +export let b = 2; +export default 4; + +if (a > c) + console.log(a, c); +else + console.log(c, a); diff --git a/test/cli/samples/stdin-with-dash/_config.js b/test/cli/samples/stdin-with-dash/_config.js new file mode 100644 index 00000000000..140aa43e535 --- /dev/null +++ b/test/cli/samples/stdin-with-dash/_config.js @@ -0,0 +1,4 @@ +module.exports = { + description: 'stdin input with dash on CLI', + command: `shx mkdir -p _actual && shx echo "0 && fail() || console.log('PASS');" | rollup - > _actual/out.js` +}; diff --git a/test/cli/samples/stdin-with-dash/_expected/out.js b/test/cli/samples/stdin-with-dash/_expected/out.js new file mode 100644 index 00000000000..80d7f8cb462 --- /dev/null +++ b/test/cli/samples/stdin-with-dash/_expected/out.js @@ -0,0 +1 @@ +console.log('PASS'); diff --git a/test/function/index.js b/test/function/index.js index 93899abba20..62dae568c39 100644 --- a/test/function/index.js +++ b/test/function/index.js @@ -177,14 +177,14 @@ runTestSuiteWithSamples('function', path.resolve(__dirname, 'samples'), (dir, co .join('\n')}` ); } - if (config.show) console.groupEnd(); - if (unintendedError) throw unintendedError; + if (config.after) config.after(); }); }); }) .catch(err => { + if (config.after) config.after(); if (config.error) { compareError(err, config.error); } else { diff --git a/test/function/samples/stdin-not-present/_config.js b/test/function/samples/stdin-not-present/_config.js new file mode 100644 index 00000000000..9223e5c89b3 --- /dev/null +++ b/test/function/samples/stdin-not-present/_config.js @@ -0,0 +1,23 @@ +const savedStdin = process.stdin; + +module.exports = { + description: 'stdin reads should fail if process.stdin not present (as in a browser)', + options: { + input: '-' + }, + before() { + delete process.stdin; + }, + after() { + process.stdin = savedStdin; + }, + error: { + // TODO we probably want a better error code here as this one is confusing + code: 'PLUGIN_ERROR', + hook: 'load', + // TODO the error message does not need to refer to browsers as there are other possible scenarios + message: 'Could not load -: stdin input is invalid in browser', + plugin: 'Rollup Core', + watchFiles: ['-'] + } +}; diff --git a/test/function/samples/stdin-not-present/main.js b/test/function/samples/stdin-not-present/main.js new file mode 100644 index 00000000000..65804ade90a --- /dev/null +++ b/test/function/samples/stdin-not-present/main.js @@ -0,0 +1 @@ +assert.equal( 1, 1 ); diff --git a/test/function/samples/stdin-read-error/_config.js b/test/function/samples/stdin-read-error/_config.js new file mode 100644 index 00000000000..0ad3314588c --- /dev/null +++ b/test/function/samples/stdin-read-error/_config.js @@ -0,0 +1,29 @@ +const { Readable } = require('stream'); +const savedStdin = process.stdin; + +module.exports = { + description: 'stdin reads should fail if process.stdin not present (as in a browser)', + options: { + input: '-' + }, + before() { + delete process.stdin; + process.stdin = new Readable({ + encoding: 'utf8', + read() { + const error = new Error('Stream is broken.'); + return this.destroy ? this.destroy(error) : this.emit('error', error); + } + }); + }, + after() { + process.stdin = savedStdin; + }, + error: { + code: 'PLUGIN_ERROR', + hook: 'load', + message: 'Could not load -: Stream is broken.', + plugin: 'Rollup Core', + watchFiles: ['-'] + } +}; diff --git a/test/function/samples/stdin-read-error/main.js b/test/function/samples/stdin-read-error/main.js new file mode 100644 index 00000000000..65804ade90a --- /dev/null +++ b/test/function/samples/stdin-read-error/main.js @@ -0,0 +1 @@ +assert.equal( 1, 1 ); diff --git a/test/function/samples/stdin-watch/_config.js b/test/function/samples/stdin-watch/_config.js new file mode 100644 index 00000000000..0eaf68e5f38 --- /dev/null +++ b/test/function/samples/stdin-watch/_config.js @@ -0,0 +1,11 @@ +module.exports = { + description: 'throws when using the "watch" option with stdin "-"', + options: { + input: '-', + watch: true + }, + error: { + // TODO add an error code + message: 'watch mode is incompatible with stdin input' + } +}; diff --git a/test/function/samples/stdin-watch/main.js b/test/function/samples/stdin-watch/main.js new file mode 100644 index 00000000000..65804ade90a --- /dev/null +++ b/test/function/samples/stdin-watch/main.js @@ -0,0 +1 @@ +assert.equal( 1, 1 ); diff --git a/test/misc/sanity-checks.js b/test/misc/sanity-checks.js index 399867535ba..7ac9a17c1e3 100644 --- a/test/misc/sanity-checks.js +++ b/test/misc/sanity-checks.js @@ -111,7 +111,7 @@ describe('sanity checks', () => { }); }); - it('throws on missing format option', () => { + it('throws on incorrect bundle.generate format option', () => { const warnings = []; return rollup @@ -122,11 +122,28 @@ describe('sanity checks', () => { }) .then(bundle => { assert.throws(() => { - bundle.generate({ file: 'x' }); + bundle.generate({ file: 'x', format: 'vanilla' }); }, /You must specify "output\.format", which can be one of "amd", "cjs", "system", "esm", "iife" or "umd"./); }); }); + it('defaults to output format `es` if not specified', () => { + const warnings = []; + + return rollup + .rollup({ + input: 'x', + plugins: [loader({ x: `export function foo(x){ console.log(x); }` })], + onwarn: warning => warnings.push(warning) + }) + .then(bundle => { + return bundle.generate({}); + }) + .then(({ output: [{ code }] }) => { + assert.equal(code, `function foo(x){ console.log(x); }\n\nexport { foo };\n`); + }); + }); + it('reuses existing error object', () => { let error;