[
https://issues.apache.org/jira/browse/CB-14140?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16515079#comment-16515079
]
ASF GitHub Bot commented on CB-14140:
-------------------------------------
raphinesse closed pull request #14: CB-14140 WIP fs-extra instead of shelljs
URL: https://github.com/apache/cordova-create/pull/14
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/index.js b/index.js
index 61181ca..30b9fcf 100644
--- a/index.js
+++ b/index.js
@@ -17,13 +17,13 @@
under the License.
*/
-var fs = require('fs');
+const fs = require('fs-extra');
+
var os = require('os');
var path = require('path');
var Promise = require('q');
var isUrl = require('is-url');
-var shell = require('shelljs');
var requireFresh = require('import-fresh');
var validateIdentifier = require('valid-identifier');
@@ -232,11 +232,11 @@ module.exports = function (dir, optionalId, optionalName,
cfg, extEvents) {
copyIfNotExists(stockAssetPath('hooks'), path.join(dir,
'hooks'));
var configXmlExists = projectConfig(dir); // moves config to
root if in www
if (!configXmlExists) {
- shell.cp(stockAssetPath('config.xml'), path.join(dir,
'config.xml'));
+ fs.copySync(stockAssetPath('config.xml'), path.join(dir,
'config.xml'));
}
} catch (e) {
if (!dirAlreadyExisted) {
- shell.rm('-rf', dir);
+ fs.removeSync(dir);
}
if (process.platform.slice(0, 3) === 'win' && e.code ===
'EPERM') {
throw new CordovaError('Symlinks on Windows require
Administrator privileges');
@@ -266,8 +266,8 @@ module.exports = function (dir, optionalId, optionalName,
cfg, extEvents) {
}
// Create basic project structure.
- shell.mkdir('-p', path.join(dir, 'platforms'));
- shell.mkdir('-p', path.join(dir, 'plugins'));
+ fs.ensureDirSync(path.join(dir, 'platforms'));
+ fs.ensureDirSync(path.join(dir, 'plugins'));
var configPath = path.join(dir, 'config.xml');
// only update config.xml if not a symlink
@@ -290,8 +290,7 @@ module.exports = function (dir, optionalId, optionalName,
cfg, extEvents) {
*/
function copyIfNotExists (src, dst) {
if (!fs.existsSync(dst) && src) {
- shell.mkdir(dst);
- shell.cp('-R', path.join(src, '*'), dst);
+ fs.copySync(src, dst);
}
}
@@ -310,7 +309,7 @@ function copyTemplateFiles (templateDir, projectDir,
isSubDir) {
// if template is a www dir
if (path.basename(templateDir) === 'www') {
copyPath = path.resolve(templateDir);
- shell.cp('-R', copyPath, projectDir);
+ fs.copySync(copyPath, path.resolve(projectDir, 'www'));
} else {
var templateFiles = fs.readdirSync(templateDir);
// Remove directories, and files that are unwanted
@@ -321,10 +320,10 @@ function copyTemplateFiles (templateDir, projectDir,
isSubDir) {
});
}
// Copy each template file after filter
- for (var i = 0; i < templateFiles.length; i++) {
- copyPath = path.resolve(templateDir, templateFiles[i]);
- shell.cp('-R', copyPath, projectDir);
- }
+ templateFiles.forEach(f => {
+ copyPath = path.resolve(templateDir, f);
+ fs.copySync(copyPath, path.resolve(projectDir, f));
+ });
}
}
@@ -373,9 +372,7 @@ function linkFromTemplate (templateDir, projectDir) {
var linkSrc, linkDst, linkFolders, copySrc, copyDst;
function rmlinkSync (src, dst, type) {
if (src && dst) {
- if (fs.existsSync(dst)) {
- shell.rm('-rf', dst);
- }
+ fs.removeSync(dst);
if (fs.existsSync(src)) {
fs.symlinkSync(src, dst, type);
}
@@ -403,7 +400,7 @@ function linkFromTemplate (templateDir, projectDir) {
// if template/www/config.xml then copy to project/config.xml
copyDst = path.join(projectDir, 'config.xml');
if (!fs.existsSync(copyDst) && fs.existsSync(copySrc)) {
- shell.cp(copySrc, projectDir);
+ fs.copySync(copySrc, copyDst);
}
}
diff --git a/package.json b/package.json
index 9ac95d1..c4e7f37 100644
--- a/package.json
+++ b/package.json
@@ -28,10 +28,10 @@
"cordova-app-hello-world": "^3.11.0",
"cordova-common": "^2.2.0",
"cordova-fetch": "^1.3.0",
+ "fs-extra": "^6.0.1",
"import-fresh": "^2.0.0",
"is-url": "^1.2.4",
"q": "^1.5.1",
- "shelljs": "^0.8.2",
"valid-identifier": "0.0.1"
},
"devDependencies": {
diff --git a/spec/create.spec.js b/spec/create.spec.js
index 68e924a..280dfbe 100644
--- a/spec/create.spec.js
+++ b/spec/create.spec.js
@@ -17,10 +17,10 @@
under the License.
*/
-var fs = require('fs');
+const fs = require('fs-extra');
+
var path = require('path');
-var shell = require('shelljs');
var requireFresh = require('import-fresh');
var create = require('..');
@@ -35,12 +35,11 @@ var project = path.join(tmpDir, appName);
// Setup and teardown test dirs
beforeEach(function () {
- shell.rm('-rf', project);
- shell.mkdir('-p', tmpDir);
+ fs.emptyDirSync(tmpDir);
});
-afterEach(function () {
+afterAll(function () {
process.chdir(path.join(__dirname, '..')); // Needed to rm the dir on
Windows.
- shell.rm('-rf', tmpDir);
+ fs.removeSync(tmpDir);
});
describe('cordova create checks for valid-identifier', function () {
@@ -57,80 +56,130 @@ describe('cordova create checks for valid-identifier',
function () {
describe('create end-to-end', function () {
- function checkProject () {
+ function checkProjectCommonArtifacts () {
// Check if top level dirs exist.
var dirs = ['hooks', 'platforms', 'plugins', 'www'];
dirs.forEach(function (d) {
expect(path.join(project, d)).toExist();
});
+ // Check that README.md exists inside of hooks
expect(path.join(project, 'hooks', 'README.md')).toExist();
- // Check if www files exist.
+ // Check that index.html exists inside of www
expect(path.join(project, 'www', 'index.html')).toExist();
- // Check that config.xml was updated.
- var configXml = new ConfigParser(path.join(project, 'config.xml'));
- expect(configXml.packageName()).toEqual(appId);
-
- // TODO (kamrik): check somehow that we got the right config.xml from
the fixture and not some place else.
- // expect(configXml.name()).toEqual('TestBase');
- }
+ // Check if config.xml exists.
+ expect(path.join(project, 'config.xml')).toExist();
- function checkConfigXml () {
- // Check if top level dirs exist.
- var dirs = ['hooks', 'platforms', 'plugins', 'www'];
- dirs.forEach(function (d) {
- expect(path.join(project, d)).toExist();
- });
- expect(path.join(project, 'hooks', 'README.md')).toExist();
-
- // index.js and template subdir folder should not exist (inner files
should be copied to the project folder)
+ // index.html, index.js and template subdir folder
+ // should not exist in top level
+ // (inner files should be copied to the project top level folder)
+ expect(path.join(project, 'index.html')).not.toExist();
expect(path.join(project, 'index.js')).not.toExist();
expect(path.join(project, 'template')).not.toExist();
- // Check if www files exist.
- expect(path.join(project, 'www', 'index.html')).toExist();
+ // Check that .gitignore does not exist inside of www
+ expect(path.join(project, 'www', '.gitignore')).not.toExist();
+
+ // Check that .npmignore does not exist inside of www
+ expect(path.join(project, 'www', '.npmignore')).not.toExist();
+
+ // Check that config.xml does not exist inside of www
+ expect(path.join(project, 'www', 'config.xml')).not.toExist();
+
+ // Check that no package.json exists inside of www
+ expect(path.join(project, 'www', 'package.json')).not.toExist();
+
+ // Check that config.xml was updated.
var configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.packageName()).toEqual(appId);
expect(configXml.version()).toEqual('1.0.0');
+ // Check that we got the right config.xml from the fixture and not
some place else.
+ expect(configXml.name()).toEqual('TestBase');
+ }
- // Check that config.xml does not exist inside of www
- expect(path.join(project, 'www', 'config.xml')).not.toExist();
+ function checkProjectArtifactsWithConfigFromTemplate () {
+ checkProjectCommonArtifacts();
+
+ // Check that standard js artifact does not exist
+ expect(path.join(project, 'www', 'js')).not.toExist();
+ expect(path.join(project, 'www', 'js', 'index.js')).not.toExist();
+
+ // [CB-12397] Check that .gitignore does not exist
+ expect(path.join(project, '.gitignore')).not.toExist();
+ // [CB-12397] Check that .npmignore does not exist
+ expect(path.join(project, '.npmignore')).not.toExist();
// Check that we got no package.json
expect(path.join(project, 'package.json')).not.toExist();
// Check that we got the right config.xml from the template and not
stock
+ const configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.description()).toEqual('this is the correct
config.xml');
}
- function checkSubDir () {
- // Check if top level dirs exist.
- var dirs = ['hooks', 'platforms', 'plugins', 'www'];
- dirs.forEach(function (d) {
- expect(path.join(project, d)).toExist();
- });
- expect(path.join(project, 'hooks', 'README.md')).toExist();
+ function checkProjectArtifactsWithNoPackageFromTemplate () {
+ checkProjectCommonArtifacts();
- // index.js and template subdir folder should not exist (inner files
should be copied to the project folder)
- expect(path.join(project, 'index.js')).not.toExist();
- expect(path.join(project, 'template')).not.toExist();
+ // Check that standard js artifact does not exist
+ expect(path.join(project, 'www', 'js')).not.toExist();
+ expect(path.join(project, 'www', 'js', 'index.js')).not.toExist();
+
+ // [CB-12397] Check that .gitignore does not exist
+ expect(path.join(project, '.gitignore')).not.toExist();
+ // [CB-12397] Check that .npmignore does not exist
+ expect(path.join(project, '.npmignore')).not.toExist();
+
+ // Check that we got no package.json
+ expect(path.join(project, 'package.json')).not.toExist();
+ }
+
+ function checkProjectArtifactsWithPackageFromTemplate () {
+ checkProjectCommonArtifacts();
+
+ // Check that standard js artifact exists
+ expect(path.join(project, 'www', 'js')).toExist();
+ expect(path.join(project, 'www', 'js', 'index.js')).toExist();
+
+ // Check if package.json exists.
+ expect(path.join(project, 'package.json')).toExist();
+
+ // [CB-12397] Check that .gitignore does not exist
+ expect(path.join(project, '.gitignore')).not.toExist();
+ // [CB-12397] Check that .npmignore exists
+ expect(path.join(project, '.npmignore')).toExist();
+
+ // Check that we got package.json (the correct one)
+ var pkjson = requireFresh(path.join(project, 'package.json'));
+ // Pkjson.displayName should equal config's name.
+ expect(pkjson.displayName).toEqual('TestBase');
+ }
+
+ function checkProjectArtifactsWithPackageFromSubDir () {
+ checkProjectCommonArtifacts();
+
+ // Check that standard js artifact does not exist
+ expect(path.join(project, 'www', 'js')).not.toExist();
+ expect(path.join(project, 'www', 'js', 'index.js')).not.toExist();
+
+ // [CB-12397] Check that .gitignore does not exist
+ expect(path.join(project, '.gitignore')).not.toExist();
+ // [CB-12397] Check that .npmignore does not exist
+ expect(path.join(project, '.npmignore')).not.toExist();
// Check if config files exist.
expect(path.join(project, 'www', 'index.html')).toExist();
- // Check that config.xml was updated.
- var configXml = new ConfigParser(path.join(project, 'config.xml'));
- expect(configXml.packageName()).toEqual(appId);
- expect(configXml.version()).toEqual('1.0.0');
// Check that we got package.json (the correct one)
var pkjson = requireFresh(path.join(project, 'package.json'));
+
// Pkjson.displayName should equal config's name.
expect(pkjson.displayName).toEqual(appName);
expect(pkjson.valid).toEqual('true');
// Check that we got the right config.xml
+ const configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.description()).toEqual('this is the correct
config.xml');
}
@@ -138,7 +187,7 @@ describe('create end-to-end', function () {
// Create a real project with no template
// use default cordova-app-hello-world app
return create(project, appId, appName, {}, events)
- .then(checkProject)
+ .then(checkProjectArtifactsWithPackageFromTemplate)
.then(function () {
var pkgJson = requireFresh(path.join(project, 'package.json'));
// confirm default hello world app copies over package.json
and it matched appId
@@ -161,7 +210,7 @@ describe('create end-to-end', function () {
expect(fetchSpy).toHaveBeenCalledTimes(1);
expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url);
})
- .then(checkProject);
+ .then(checkProjectArtifactsWithPackageFromTemplate);
});
it('should successfully run with NPM package', function () {
@@ -179,7 +228,7 @@ describe('create end-to-end', function () {
expect(fetchSpy).toHaveBeenCalledTimes(1);
expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url);
})
- .then(checkProject);
+ .then(checkProjectArtifactsWithPackageFromTemplate);
});
it('should successfully run with NPM package and explicitly fetch latest
if no version is given', function () {
@@ -198,7 +247,7 @@ describe('create end-to-end', function () {
expect(fetchSpy).toHaveBeenCalledTimes(1);
expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url +
'@latest');
})
- .then(checkProject);
+ .then(checkProjectArtifactsWithPackageFromTemplate);
});
it('should successfully run with template not having a package.json at
toplevel', function () {
@@ -211,7 +260,7 @@ describe('create end-to-end', function () {
}
};
return create(project, appId, appName, config, events)
- .then(checkProject)
+ .then(checkProjectArtifactsWithNoPackageFromTemplate)
.then(function () {
// Check that we got the right config.xml
var configXml = new ConfigParser(path.join(project,
'config.xml'));
@@ -229,7 +278,7 @@ describe('create end-to-end', function () {
}
};
return create(project, appId, appName, config, events)
- .then(checkProject);
+ .then(checkProjectArtifactsWithNoPackageFromTemplate);
});
it('should successfully run with template having package.json, and
subdirectory, and no package.json in subdirectory', function () {
@@ -242,7 +291,7 @@ describe('create end-to-end', function () {
}
};
return create(project, appId, appName, config, events)
- .then(checkProject);
+ .then(checkProjectArtifactsWithNoPackageFromTemplate);
});
it('should successfully run with template having package.json, and
subdirectory, and package.json in subdirectory', function () {
@@ -255,7 +304,7 @@ describe('create end-to-end', function () {
}
};
return create(project, appId, appName, config, events)
- .then(checkSubDir);
+ .then(checkProjectArtifactsWithPackageFromSubDir);
});
it('should successfully run config.xml in the www folder and move it
outside', function () {
@@ -268,7 +317,7 @@ describe('create end-to-end', function () {
}
};
return create(project, appId, appName, config, events)
- .then(checkConfigXml);
+ .then(checkProjectArtifactsWithConfigFromTemplate);
});
it('should successfully run with www folder as the template', function () {
@@ -281,13 +330,20 @@ describe('create end-to-end', function () {
}
};
return create(project, appId, appName, config, events)
- .then(checkConfigXml);
+ .then(checkProjectArtifactsWithConfigFromTemplate)
+ .then(() => {
+ // Additional check that we have the fixture www,
+ // not one from stock the app
+ expect(path.join(project, 'www',
'fixture-marker-page.html')).toExist();
+ expect(path.join(project, 'www', 'subdir')).toExist();
+ expect(path.join(project, 'www', 'subdir',
'sub-fixture-marker-page.html')).toExist();
+ });
});
it('should successfully run with existing, empty destination', function ()
{
- shell.mkdir('-p', project);
+ fs.ensureDirSync(project);
return create(project, appId, appName, {}, events)
- .then(checkProject);
+ .then(checkProjectArtifactsWithPackageFromTemplate);
});
describe('when --link-to is provided', function () {
@@ -315,6 +371,7 @@ describe('create end-to-end', function () {
// Check www/config exists
expect(path.join(project, 'www', 'config.xml')).toExist();
+
// Check www/config.xml was not updated.
var configXml = new ConfigParser(path.join(project, 'www',
'config.xml'));
expect(configXml.packageName()).toEqual('io.cordova.hellocordova');
@@ -323,6 +380,7 @@ describe('create end-to-end', function () {
// Check that config.xml was copied to project/config.xml
expect(path.join(project, 'config.xml')).toExist();
+
configXml = new ConfigParser(path.join(project, 'config.xml'));
expect(configXml.description()).toEqual('this is the correct
config.xml');
// Check project/config.xml was updated.
diff --git a/spec/helpers.js b/spec/helpers.js
index daf6722..bc48248 100644
--- a/spec/helpers.js
+++ b/spec/helpers.js
@@ -17,12 +17,11 @@
under the License.
*/
-const fs = require('fs');
+const fs = require('fs-extra');
const os = require('os');
const path = require('path');
const rewire = require('rewire');
-const shell = require('shelljs');
// Disable regular console output during tests
const CordovaLogger = require('cordova-common').CordovaLogger;
@@ -44,7 +43,7 @@ function createWithMockFetch (dir, id, name, cfg, events) {
const fetchSpy = jasmine.createSpy('fetchSpy')
.and.callFake(() => Promise.resolve(mockFetchDest));
- shell.cp('-R', templateDir, mockFetchDest);
+ fs.copySync(templateDir, mockFetchDest);
return createWith({fetch: fetchSpy})(dir, id, name, cfg, events)
.then(() => fetchSpy);
}
@@ -85,9 +84,9 @@ beforeEach(function () {
result.pass = fs.existsSync(testPath);
if (result.pass) {
- result.message = 'Expected file ' + testPath + ' to
exist.';
- } else {
result.message = 'Expected file ' + testPath + ' to
not exist.';
+ } else {
+ result.message = 'Expected file ' + testPath + ' to
exist.';
}
return result;
diff --git a/spec/templates/config_in_www/www/fixture-marker-page.html
b/spec/templates/config_in_www/www/fixture-marker-page.html
new file mode 100644
index 0000000..6a9821c
--- /dev/null
+++ b/spec/templates/config_in_www/www/fixture-marker-page.html
@@ -0,0 +1 @@
+<h1>Fixture marker page</h1>
diff --git
a/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html
b/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html
new file mode 100644
index 0000000..311ec82
--- /dev/null
+++ b/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html
@@ -0,0 +1 @@
+<h1>Sub-fixture marker page</h1>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Use fs-extra (and which) instead of shelljs
> -------------------------------------------
>
> Key: CB-14140
> URL: https://issues.apache.org/jira/browse/CB-14140
> Project: Apache Cordova
> Issue Type: Improvement
> Components: cordova-android, cordova-common, cordova-create,
> cordova-fetch, cordova-ios, cordova-lib, cordova-osx, cordova-windows
> Reporter: Chris Brody
> Assignee: Darryl Pogue
> Priority: Minor
>
> It is more efficient to use fs-extra, sometimes with some help from which,
> than shelljs.
> This improvement has already landed in the master branch of cordova-common
> for the next major release, and work has already been started in some other
> packages.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org
