From c58a72923463f26aa71d73cb3f500531e2923394 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 00:15:32 +0800 Subject: [PATCH 1/7] refactor: use jscodeshift instead of recast to inject imports and options fixes #3309 --- packages/@vue/cli/__tests__/Generator.spec.js | 21 +++++++- .../cli/lib/util/injectImportsAndOptions.js | 50 ++++++++++--------- packages/@vue/cli/package.json | 1 + 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/packages/@vue/cli/__tests__/Generator.spec.js b/packages/@vue/cli/__tests__/Generator.spec.js index 5091601959..458d9deecf 100644 --- a/packages/@vue/cli/__tests__/Generator.spec.js +++ b/packages/@vue/cli/__tests__/Generator.spec.js @@ -22,6 +22,7 @@ new Vue({ render: h => h(App) }).$mount('#app') `.trim()) +fs.writeFileSync(path.resolve(templateDir, 'empty-entry.js'), `;`) // replace stubs fs.writeFileSync(path.resolve(templateDir, 'replace.js'), ` @@ -465,10 +466,28 @@ test('api: addEntryImport & addEntryInjection', async () => { ] }) await generator.generate() - expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\s+import bar from 'bar'/) + expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\nimport bar from 'bar'/) expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/new Vue\({\s+p: p\(\),\s+baz,\s+foo,\s+bar,\s+render: h => h\(App\)\s+}\)/) }) +test('api: injectImports to empty file', async () => { + const generator = new Generator('/', { plugins: [ + { + id: 'test', + apply: api => { + api.injectImports('main.js', `import foo from 'foo'`) + api.injectImports('main.js', `import bar from 'bar'`) + api.render({ + 'main.js': path.join(templateDir, 'empty-entry.js') + }) + } + } + ] }) + + await generator.generate() + expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\nimport bar from 'bar'/) +}) + test('api: addEntryDuplicateImport', async () => { const generator = new Generator('/', { plugins: [ { diff --git a/packages/@vue/cli/lib/util/injectImportsAndOptions.js b/packages/@vue/cli/lib/util/injectImportsAndOptions.js index 68e69958aa..5df15ca067 100644 --- a/packages/@vue/cli/lib/util/injectImportsAndOptions.js +++ b/packages/@vue/cli/lib/util/injectImportsAndOptions.js @@ -9,40 +9,42 @@ module.exports = function injectImportsAndOptions (source, imports, injections) return source } - const recast = require('recast') - const ast = recast.parse(source) + const j = require('jscodeshift') if (hasImports) { - const toImport = i => recast.parse(`${i}\n`).program.body[0] - const importDeclarations = [] - let lastImportIndex = -1 + const root = j(source) - recast.types.visit(ast, { - visitImportDeclaration ({ node }) { - lastImportIndex = ast.program.body.findIndex(n => n === node) - importDeclarations.push(node) - return false - } + const stringToNode = i => j(`${i}\n`).nodes()[0].program.body[0] + const nodeToHash = node => JSON.stringify({ + specifiers: node.specifiers.map(s => s.local.name), + source: node.source.raw }) - // avoid blank line after the previous import - if (lastImportIndex !== -1) { - delete ast.program.body[lastImportIndex].loc - } - const nonDuplicates = i => { - return !importDeclarations.some(node => { - const result = node.source.raw === i.source.raw && node.specifiers.length === i.specifiers.length + const importSet = new Set() + root.find(j.ImportDeclaration) + .forEach(p => importSet.add(nodeToHash(p.value))) + const nonDuplicates = node => !importSet.has(nodeToHash(node)) + + const importNodes = imports.map(stringToNode).filter(nonDuplicates) - return result && node.specifiers.every((item, index) => { - return i.specifiers[index].local.name === item.local.name - }) - }) + if (importSet.size) { + root.find(j.ImportDeclaration) + .at(-1) + // a tricky way to avoid blank line after the previous import + .forEach(n => delete n.value.loc) + .insertAfter(importNodes) + } else { + // no pre-existing import declarations + const { body } = root.get().node.program + body.unshift.apply(body, importNodes) } - const newImports = imports.map(toImport).filter(nonDuplicates) - ast.program.body.splice(lastImportIndex + 1, 0, ...newImports) + source = root.toSource() } + const recast = require('recast') + const ast = recast.parse(source) + if (hasInjections) { const toProperty = i => { return recast.parse(`({${i}})`).program.body[0].expression.properties diff --git a/packages/@vue/cli/package.json b/packages/@vue/cli/package.json index ab348dfeec..3685055423 100644 --- a/packages/@vue/cli/package.json +++ b/packages/@vue/cli/package.json @@ -45,6 +45,7 @@ "isbinaryfile": "^3.0.2", "javascript-stringify": "^1.6.0", "js-yaml": "^3.13.1", + "jscodeshift": "^0.6.4", "lodash.clonedeep": "^4.5.0", "minimist": "^1.2.0", "recast": "^0.17.6", From 979c9cf4b13f69dd65af6c8e775a7d4e603ddb73 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 00:21:47 +0800 Subject: [PATCH 2/7] refactor: n => p --- packages/@vue/cli/lib/util/injectImportsAndOptions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@vue/cli/lib/util/injectImportsAndOptions.js b/packages/@vue/cli/lib/util/injectImportsAndOptions.js index 5df15ca067..7da0672922 100644 --- a/packages/@vue/cli/lib/util/injectImportsAndOptions.js +++ b/packages/@vue/cli/lib/util/injectImportsAndOptions.js @@ -31,7 +31,7 @@ module.exports = function injectImportsAndOptions (source, imports, injections) root.find(j.ImportDeclaration) .at(-1) // a tricky way to avoid blank line after the previous import - .forEach(n => delete n.value.loc) + .forEach(p => delete p.value.loc) .insertAfter(importNodes) } else { // no pre-existing import declarations From b4084b1d0d79abc198e9dda2aabc4fc70bfee747 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 00:31:22 +0800 Subject: [PATCH 3/7] refactor: use spread syntax --- packages/@vue/cli/lib/util/injectImportsAndOptions.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@vue/cli/lib/util/injectImportsAndOptions.js b/packages/@vue/cli/lib/util/injectImportsAndOptions.js index 7da0672922..cdda168e4e 100644 --- a/packages/@vue/cli/lib/util/injectImportsAndOptions.js +++ b/packages/@vue/cli/lib/util/injectImportsAndOptions.js @@ -35,8 +35,7 @@ module.exports = function injectImportsAndOptions (source, imports, injections) .insertAfter(importNodes) } else { // no pre-existing import declarations - const { body } = root.get().node.program - body.unshift.apply(body, importNodes) + root.get().node.program.body.unshift(...importNodes) } source = root.toSource() From 2a491ade7257c47d898244b8d9337f5e504440a4 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 00:55:56 +0800 Subject: [PATCH 4/7] refactor: migrate injectRootOptions --- .../cli/lib/util/injectImportsAndOptions.js | 72 +++++++++---------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/packages/@vue/cli/lib/util/injectImportsAndOptions.js b/packages/@vue/cli/lib/util/injectImportsAndOptions.js index cdda168e4e..b66f972867 100644 --- a/packages/@vue/cli/lib/util/injectImportsAndOptions.js +++ b/packages/@vue/cli/lib/util/injectImportsAndOptions.js @@ -10,67 +10,61 @@ module.exports = function injectImportsAndOptions (source, imports, injections) } const j = require('jscodeshift') + const root = j(source) if (hasImports) { - const root = j(source) - - const stringToNode = i => j(`${i}\n`).nodes()[0].program.body[0] - const nodeToHash = node => JSON.stringify({ - specifiers: node.specifiers.map(s => s.local.name), - source: node.source.raw + const toASTNode = i => j(`${i}\n`).nodes()[0].program.body[0] + const toHash = importASTNode => JSON.stringify({ + specifiers: importASTNode.specifiers.map(s => s.local.name), + source: importASTNode.source.raw }) const importSet = new Set() root.find(j.ImportDeclaration) - .forEach(p => importSet.add(nodeToHash(p.value))) - const nonDuplicates = node => !importSet.has(nodeToHash(node)) + .forEach(({ node }) => importSet.add(toHash(node))) + const nonDuplicates = node => !importSet.has(toHash(node)) - const importNodes = imports.map(stringToNode).filter(nonDuplicates) + const importASTNodes = imports.map(toASTNode).filter(nonDuplicates) if (importSet.size) { root.find(j.ImportDeclaration) .at(-1) // a tricky way to avoid blank line after the previous import - .forEach(p => delete p.value.loc) - .insertAfter(importNodes) + .forEach(({ node }) => delete node.loc) + .insertAfter(importASTNodes) } else { // no pre-existing import declarations - root.get().node.program.body.unshift(...importNodes) + root.get().node.program.body.unshift(...importASTNodes) } - - source = root.toSource() } - const recast = require('recast') - const ast = recast.parse(source) - if (hasInjections) { const toProperty = i => { - return recast.parse(`({${i}})`).program.body[0].expression.properties + return j(`({${i}})`).nodes()[0].program.body[0].expression.properties } - recast.types.visit(ast, { - visitNewExpression ({ node }) { - if (node.callee.name === 'Vue') { - const options = node.arguments[0] - if (options && options.type === 'ObjectExpression') { - const nonDuplicates = i => { - return !options.properties.slice(0, -1).some(p => { - return p.key.name === i[0].key.name && - recast.print(p.value).code === recast.print(i[0].value).code - }) - } - // inject at index length - 1 as it's usually the render fn - options.properties = [ - ...options.properties.slice(0, -1), - ...([].concat(...injections.map(toProperty).filter(nonDuplicates))), - ...options.properties.slice(-1) - ] + + root + .find(j.NewExpression, { + callee: { name: 'Vue' } + }) + .forEach(({ node }) => { + const options = node.arguments[0] + if (options && options.type === 'ObjectExpression') { + const nonDuplicates = i => { + return !options.properties.slice(0, -1).some(p => { + return p.key.name === i[0].key.name && + j(p.value).toSource() === j(i[0].value).toSource() + }) } + // inject at index length - 1 as it's usually the render fn + options.properties = [ + ...options.properties.slice(0, -1), + ...([].concat(...injections.map(toProperty).filter(nonDuplicates))), + ...options.properties.slice(-1) + ] } - return false - } - }) + }) } - return recast.print(ast).code + return root.toSource() } From 12e90ddad707a74a4618bc9f17b5a390e86346fa Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 09:02:58 +0800 Subject: [PATCH 5/7] test: fix windows newline --- packages/@vue/cli/__tests__/Generator.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@vue/cli/__tests__/Generator.spec.js b/packages/@vue/cli/__tests__/Generator.spec.js index 458d9deecf..7e4e0cb274 100644 --- a/packages/@vue/cli/__tests__/Generator.spec.js +++ b/packages/@vue/cli/__tests__/Generator.spec.js @@ -466,7 +466,7 @@ test('api: addEntryImport & addEntryInjection', async () => { ] }) await generator.generate() - expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\nimport bar from 'bar'/) + expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\r?\nimport bar from 'bar'/) expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/new Vue\({\s+p: p\(\),\s+baz,\s+foo,\s+bar,\s+render: h => h\(App\)\s+}\)/) }) @@ -485,7 +485,7 @@ test('api: injectImports to empty file', async () => { ] }) await generator.generate() - expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\nimport bar from 'bar'/) + expect(fs.readFileSync('/main.js', 'utf-8')).toMatch(/import foo from 'foo'\r?\nimport bar from 'bar'/) }) test('api: addEntryDuplicateImport', async () => { From 63095789328b5f820c32153b08c48f709b128228 Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 16:26:17 +0800 Subject: [PATCH 6/7] refactor: make the code more succinct --- .../cli/lib/util/injectImportsAndOptions.js | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/packages/@vue/cli/lib/util/injectImportsAndOptions.js b/packages/@vue/cli/lib/util/injectImportsAndOptions.js index b66f972867..8a000f8e73 100644 --- a/packages/@vue/cli/lib/util/injectImportsAndOptions.js +++ b/packages/@vue/cli/lib/util/injectImportsAndOptions.js @@ -13,21 +13,20 @@ module.exports = function injectImportsAndOptions (source, imports, injections) const root = j(source) if (hasImports) { - const toASTNode = i => j(`${i}\n`).nodes()[0].program.body[0] - const toHash = importASTNode => JSON.stringify({ - specifiers: importASTNode.specifiers.map(s => s.local.name), - source: importASTNode.source.raw + const toImportAST = i => j(`${i}\n`).nodes()[0].program.body[0] + const toImportHash = node => JSON.stringify({ + specifiers: node.specifiers.map(s => s.local.name), + source: node.source.raw }) - const importSet = new Set() - root.find(j.ImportDeclaration) - .forEach(({ node }) => importSet.add(toHash(node))) - const nonDuplicates = node => !importSet.has(toHash(node)) + const declarations = root.find(j.ImportDeclaration) + const importSet = new Set(declarations.nodes().map(toImportHash)) + const nonDuplicates = node => !importSet.has(toImportHash(node)) - const importASTNodes = imports.map(toASTNode).filter(nonDuplicates) + const importASTNodes = imports.map(toImportAST).filter(nonDuplicates) - if (importSet.size) { - root.find(j.ImportDeclaration) + if (declarations.length) { + declarations .at(-1) // a tricky way to avoid blank line after the previous import .forEach(({ node }) => delete node.loc) @@ -39,31 +38,24 @@ module.exports = function injectImportsAndOptions (source, imports, injections) } if (hasInjections) { - const toProperty = i => { - return j(`({${i}})`).nodes()[0].program.body[0].expression.properties + const toPropertyAST = i => { + return j(`({${i}})`).nodes()[0].program.body[0].expression.properties[0] } - root + const options = root .find(j.NewExpression, { - callee: { name: 'Vue' } - }) - .forEach(({ node }) => { - const options = node.arguments[0] - if (options && options.type === 'ObjectExpression') { - const nonDuplicates = i => { - return !options.properties.slice(0, -1).some(p => { - return p.key.name === i[0].key.name && - j(p.value).toSource() === j(i[0].value).toSource() - }) - } - // inject at index length - 1 as it's usually the render fn - options.properties = [ - ...options.properties.slice(0, -1), - ...([].concat(...injections.map(toProperty).filter(nonDuplicates))), - ...options.properties.slice(-1) - ] - } + callee: { name: 'Vue' }, + arguments: [{ type: 'ObjectExpression' }] }) + .map(path => path.get('arguments', 0)) + const { properties } = options.get().node + + const toPropertyHash = p => `${p.key.name}: ${j(p.value).toSource()}` + const propertySet = new Set(properties.map(toPropertyHash)) + const nonDuplicates = p => !propertySet.has(toPropertyHash(p)) + + // inject at index length - 1 as it's usually the render fn + properties.splice(-1, 0, ...injections.map(toPropertyAST).filter(nonDuplicates)) } return root.toSource() From 850a662009b2cddf7d75380b855aa6bbe85baa7d Mon Sep 17 00:00:00 2001 From: Haoqun Jiang Date: Thu, 16 May 2019 16:42:53 +0800 Subject: [PATCH 7/7] refactor: no need to declare an `option` variable --- packages/@vue/cli/lib/util/injectImportsAndOptions.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/@vue/cli/lib/util/injectImportsAndOptions.js b/packages/@vue/cli/lib/util/injectImportsAndOptions.js index 8a000f8e73..36a94622c0 100644 --- a/packages/@vue/cli/lib/util/injectImportsAndOptions.js +++ b/packages/@vue/cli/lib/util/injectImportsAndOptions.js @@ -42,13 +42,15 @@ module.exports = function injectImportsAndOptions (source, imports, injections) return j(`({${i}})`).nodes()[0].program.body[0].expression.properties[0] } - const options = root + const properties = root .find(j.NewExpression, { callee: { name: 'Vue' }, arguments: [{ type: 'ObjectExpression' }] }) .map(path => path.get('arguments', 0)) - const { properties } = options.get().node + .get() + .node + .properties const toPropertyHash = p => `${p.key.name}: ${j(p.value).toSource()}` const propertySet = new Set(properties.map(toPropertyHash))