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.

Sign up for GitHub

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

Jump to bottom

cli(entry): quotes sanitization #337

Merged
merged 7 commits into from
Apr 17, 2018
Merged

cli(entry): quotes sanitization #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
90269a5
chore: minor doc fixes
dhruvdutt Mar 11, 2018
27ffd25
cli(entry): quotes sanitization
dhruvdutt Mar 11, 2018
5e01249
tests(entry): add test case for double quotes
dhruvdutt Mar 11, 2018
9e9f162
tests(entry): update snapshots
dhruvdutt Mar 11, 2018
2a16c43
misc(utils): entry - variable parity
dhruvdutt Mar 14, 2018
7961afb
misc(prop-types): sort
dhruvdutt Mar 14, 2018
da851a0
cli(entry): multiple entries quotes sanitization
dhruvdutt Mar 14, 2018
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
4 changes: 2 additions & 2 deletions lib/commands/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ const modifyHelper = require("../utils/modify-config-helper");
* First function to be called after running the init flag. This is a check,
* if we are running the init command with no arguments or if we got dependencies
*
* @param {Array} args - array of arguments such as
* @param {Array} args - array of arguments such as
* packages included when running the init command
* @returns {Function} creator/npmPackagesExists - returns an installation of the package,
* @returns {Function} creator/npmPackagesExists - returns an installation of the package,
* followed up with a yeoman instance of that if there's packages. If not, it creates a defaultGenerator
*/

Expand Down
34 changes: 19 additions & 15 deletions lib/generators/utils/entry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ const validate = require("./validate");
*
* Prompts for entry points, either if it has multiple or one entry
*
* @param {Object} self - A variable holding the instance of the prompting
* @param {Object} answer - Previous answer from asking if the user wants single or multiple entries
* @returns {Object} An Object that holds the answers given by the user, later used to scaffold
* @param {Object} self - A variable holding the instance of the prompting
* @param {Object} answer - Previous answer from asking if the user wants single or multiple entries
* @returns {Object} An Object that holds the answers given by the user, later used to scaffold
*/

module.exports = (self, answer) => {
Expand All @@ -28,7 +28,7 @@ module.exports = (self, answer) => {
let webpackEntryPoint = {};
entryIdentifiers = multipleEntriesAnswer["multipleEntries"].split(",");
function forEachPromise(obj, fn) {
return obj.reduce(function(promise, prop) {
return obj.reduce((promise, prop) => {
const trimmedProp = prop.trim();
return promise.then(n => {
if (n) {
Expand All @@ -40,7 +40,7 @@ module.exports = (self, answer) => {
!n[val].includes("path") &&
!n[val].includes("process")
) {
n[val] = `"'${n[val]}.js'"`;
n[val] = `\'${n[val].replace(/"|'/g,"").concat(".js")}\'`;
}
webpackEntryPoint[val] = n[val];
});
Expand All @@ -59,18 +59,18 @@ module.exports = (self, answer) => {
validate
)
])
).then(propAns => {
Object.keys(propAns).forEach(val => {
).then(entryPropAnswer => {
Object.keys(entryPropAnswer).forEach(val => {
if (
propAns[val].charAt(0) !== "(" &&
propAns[val].charAt(0) !== "[" &&
!propAns[val].includes("function") &&
!propAns[val].includes("path") &&
!propAns[val].includes("process")
entryPropAnswer[val].charAt(0) !== "(" &&
entryPropAnswer[val].charAt(0) !== "[" &&
!entryPropAnswer[val].includes("function") &&
!entryPropAnswer[val].includes("path") &&
!entryPropAnswer[val].includes("process")
) {
propAns[val] = `"'${propAns[val]}.js'"`;
entryPropAnswer[val] = `\'${entryPropAnswer[val].replace(/"|'/g,"").concat(".js")}\'`;
}
webpackEntryPoint[val] = propAns[val];
webpackEntryPoint[val] = entryPropAnswer[val];
});
return webpackEntryPoint;
});
Expand All @@ -83,7 +83,11 @@ module.exports = (self, answer) => {
"Which module will be the first to enter the application? [default: ./src/index]"
)
])
.then(singularAnswer => `"${singularAnswer["singularEntry"]}"`);
.then(singularEntryAnswer => {
let { singularEntry } = singularEntryAnswer;
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Member Author

@dhruvdutt dhruvdutt Mar 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Testing if input has " and then swapping it with '.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at the test, probably to check if entry point name has \ ?

if (singularEntry.indexOf("\"") >= 0) singularEntry = singularEntry.replace(/"/g, "'");
return singularEntry;
});
}
return result;
};
8 changes: 4 additions & 4 deletions lib/init/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ const transformsObject = {
* Maps back transforms that needs to be run using the configuration
* provided.
*
* @param {Object} transformObject - An Object with all transformations
* @param {Object} config - Configuration to transform
* @param {Object} transformObject - An Object with all transformations
* @param {Object} config - Configuration to transform
* @returns {Object} - An Object with the transformations to be run
*/

Expand Down Expand Up @@ -104,8 +104,8 @@ function mapOptionsToTransform(transformObject, config) {
*
* Runs the transformations from an object we get from yeoman
*
* @param {Object} webpackProperties - Configuration to transform
* @param {String} action - Action to be done on the given ast
* @param {Object} webpackProperties - Configuration to transform
* @param {String} action - Action to be done on the given ast
* @returns {Promise} - A promise that writes each transform, runs prettier
* and writes the file
*/
Expand Down
27 changes: 17 additions & 10 deletions lib/init/transformations/entry/__snapshots__/entry.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,19 @@ exports[`entry transforms correctly using "entry-0" data 1`] = `

exports[`entry transforms correctly using "entry-0" data 2`] = `
"module.exports = {
entry: ['index.js', 'app.js']
entry: \\"index.js\\"
}
"
`;

exports[`entry transforms correctly using "entry-0" data 3`] = `
"module.exports = {
entry: ['index.js', 'app.js']
}
"
`;

exports[`entry transforms correctly using "entry-0" data 4`] = `
"module.exports = {
entry: {
index: 'index.js',
Expand All @@ -24,7 +31,7 @@ exports[`entry transforms correctly using "entry-0" data 3`] = `
"
`;

exports[`entry transforms correctly using "entry-0" data 4`] = `
exports[`entry transforms correctly using "entry-0" data 5`] = `
"module.exports = {
entry: {
something,
Expand All @@ -35,42 +42,42 @@ exports[`entry transforms correctly using "entry-0" data 4`] = `
"
`;

exports[`entry transforms correctly using "entry-0" data 5`] = `
exports[`entry transforms correctly using "entry-0" data 6`] = `
"module.exports = {
entry: () => 'index.js'
}
"
`;

exports[`entry transforms correctly using "entry-0" data 6`] = `
exports[`entry transforms correctly using "entry-0" data 7`] = `
"module.exports = {
entry: () => new Promise((resolve) => resolve(['./app', './router']))
}
"
`;

exports[`entry transforms correctly using "entry-0" data 7`] = `
exports[`entry transforms correctly using "entry-0" data 8`] = `
"module.exports = {
entry: entryStringVariable
}
"
`;

exports[`entry transforms correctly using "entry-0" data 8`] = `
exports[`entry transforms correctly using "entry-0" data 9`] = `
"module.exports = {
entry: 'index.js'
}
"
`;

exports[`entry transforms correctly using "entry-0" data 9`] = `
exports[`entry transforms correctly using "entry-0" data 10`] = `
"module.exports = {
entry: ['index.js', 'app.js']
}
"
`;

exports[`entry transforms correctly using "entry-0" data 10`] = `
exports[`entry transforms correctly using "entry-0" data 11`] = `
"module.exports = {
entry: {
something,
Expand All @@ -81,14 +88,14 @@ exports[`entry transforms correctly using "entry-0" data 10`] = `
"
`;

exports[`entry transforms correctly using "entry-0" data 11`] = `
exports[`entry transforms correctly using "entry-0" data 12`] = `
"module.exports = {
entry: () => new Promise((resolve) => resolve(['./app', './router']))
}
"
`;

exports[`entry transforms correctly using "entry-0" data 12`] = `
exports[`entry transforms correctly using "entry-0" data 13`] = `
"module.exports = {
entry: entryStringVariable
}
Expand Down
10 changes: 5 additions & 5 deletions lib/init/transformations/entry/entry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ const utils = require("../../../utils/ast-utils");
* Transform for entry. Finds the entry property from yeoman and creates a
* property based on what the user has given us.
*
* @param j — jscodeshift API
* @param ast - jscodeshift API
* @param {any} webpackProperties - transformation object to scaffold
* @param {String} action - action that indicates what to be done to the AST
* @returns ast - jscodeshift API
* @param j - jscodeshift API
* @param ast - jscodeshift API
* @param {any} webpackProperties - transformation object to scaffold
* @param {String} action - action that indicates what to be done to the AST
* @returns ast - jscodeshift API
*/

module.exports = function entryTransform(j, ast, webpackProperties, action) {
Expand Down
1 change: 1 addition & 0 deletions lib/init/transformations/entry/entry.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const defineTest = require("../../../utils/defineTest");

defineTest(__dirname, "entry", "entry-0", "'index.js'", "init");
defineTest(__dirname, "entry", "entry-0", "\"index.js\"", "init");
defineTest(__dirname, "entry", "entry-0", ["'index.js'", "'app.js'"], "init");
defineTest(
__dirname,
Expand Down
24 changes: 12 additions & 12 deletions lib/utils/prop-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,30 @@
*/

module.exports = new Set([
"amd",
"bail",
"cache",
"context",
"devServer",
"devtool",
"entry",
"externals",
"merge",
"mode",
"module",
"node",
"output",
"parallelism",
"performance",
"plugins",
"resolve",
"target",
"watch",
"watchOptions",
"stats",
"mode",
"amd",
"bail",
"cache",
"profile",
"merge",
"parallelism",
"recordsInputPath",
"recordsOutputPath",
"recordsPath",
"resolveLoader"
"resolve",
Copy link
Member

Choose a reason for hiding this comment

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

Bad rebase?

Copy link
Member Author

Choose a reason for hiding this comment

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

It includes all properties. I think Git/GitHub diff might not interpret it as sorting but only show changes in code based on line number. That's why we see such confusing diffs. You can verify it from the current file.

"resolveLoader",
"stats",
"target",
"watch",
"watchOptions",
]);