From @raphinesse on July 5, 2018 8:47

Some files and dirs are copied to dest from cordova-app-hello-world if they
are not present in the custom template being used.

Current situation

These files and dirs are:

• www
• hooks
• config.xml

Pain Points

• File list is outdated (hooks dir has been deprecated, package.json is missing)
• Template authors might be surprised to see files they did not use

Proposal

KISS and remove this behavior

• Makes cordovaCreate more agnostic to the template layout
• Principle of least surprise for template authors
• Less maintenance

Evolved from apache/cordova-discuss#89. Possibly related to apache/cordova-discuss#78

Copied from original issue: apache/cordova-discuss#102

From @raphinesse on July 5, 2018 8:49

Current situation

cordovaCreate supports handling of at least three different layouts for the
templates that are referenced by url. Let TEMPLATE be the directory on disk that
contains the resolved template. Then there are these three cases:

1. TEMPLATE contains a proper template as documented in the template docs
   cordovaCreate will copy all contents of TEMPLATE/template_src to dir (simplified)
2. A "bare" folder w/out a main module
   cordovaCreate will copy all contents of TEMPLATE except for some blacklisted² files to dir
3. basename(TEMPLATE) === 'www'
cordovaCreate will copy TEMPLATE to dir/www

Additionally cordovaCreate still handles (and corrects) the case that
config.xml is located in TEMPLATE/www instead of TEMPLATE.

²: package.json, RELEASENOTES.md, .git, NOTICE, LICENSE, COPYRIGHT, .npmignore

Pain Points

• AFAICT, only case 1 is documented
• Increased code & documentation complexity

Proposal

• Drop support for 3.
• Document or drop 2.
• Drop Support for templates with www/config.xml.

Migrated from apache/cordova-discuss#89

Copied from original issue: apache/cordova-discuss#103

From @raphinesse on July 5, 2018 9:4

Current situation

Ignoring all special cases discussed in #103, cordovaCreate w/ enabled link option will

• symlink folders www, merges and hooks
• symlink the file config.xml

Pain Points

• The current implementation does not seem to solve any valid use cases
• This is a major impediment to a sane implementation of #69. Especially the need for renaming to .gitingore in cordova-create (#8), is heavily conflicting with the linking feature.
• Needs special privileges on Windows since it also links files.
• hooks is deprecated

Proposal

Drop it. 🔥

It seems it was planned to remove --link-to before apache/cordova-discuss#49 (comment). Support was then removed in apache/cordova-lib#456 and re-added in 79cace5 addressing https://issues.apache.org/jira/browse/CB-11623 which is still open because the current implementation apparently still not satisfies the reporter's requirements. 😒

The reporter's requirements are to create a cordova project with www linking to some existing web app. This could be easily achieved by running

$ cordova create bin com.example.domain APPNAME
$ rm -r bin/www && ln -sr www bin/www

instead of

$ cordova create bin com.example.domain APPNAME --link-to=www

Evolved from apache/cordova-discuss#89

Copied from original issue: apache/cordova-discuss#104

From @raphinesse on July 5, 2018 8:25

I'm talking about the logic behind setting the appropriate fields for App ID,
App Name and App Version in the files package.json and config.xml.

Current situation

The most common strategy is to set the attribute in both files iff a value for
it was given to cordovaCreate. Exceptions are

• App ID in package.json: set attribute if value given else set to helloworld
• App Version in both files: always set to 1.0.0

Pain Points

• Strategies employed for setting the attributes differ
• Falling back to hard coded defaults is not very flexible (e.g. I usually start my version numbering at 0.1.0)

Proposal

For every file f and every attribute attr, do the following

if (attr in opts) {
    // Save attribute value passed to cordovaCreate to f
    f[attr] = opts[attr]
} else if (attr in f) {
    // Attribute already present in f and no override specified
    // => Leave the existing value untouched
} else if (isRequired(attr, f)) {
    handleMissingRequiredAttribute(attr, f)
}

where isRequired would have to be defined adequately
and handleMissingRequiredAttribute could either:

1. Throw an error (my preference)
2. Save some hard coded fallback value to f (possibly warn that f is invalid)

Related issues

CB-12274 - widget version number not copied from template config.xml file

Migrated from apache/cordova-discuss#89

Copied from original issue: apache/cordova-discuss#100

Bug Report

Problem

The tmp package is required at ^0.2.1 by cordova-create, and this effectively breaks cordova 11 (maybe others?) on node 12. Yes, that is an ancient version of node, but here we are.

What is expected to happen?

Installation of cordova 11 on node 12 should install without errors.

What does actually happen?

The following error is thrown:
npm ERR! code ENOTSUP
npm ERR! notsup Unsupported engine for [email protected]: wanted: {"node":">=14.14"} (current: {"node":"12.22.12","npm":"6.14.16"})
npm ERR! notsup Not compatible with your version of node/npm: [email protected]
npm ERR! ..

Information

This can be overridden locally with an overrides entry in package.json, but tightening up the version of tmp used to "tmp": "0.2.1" would be helpful since "^0.2.1" allows the breaking change in.

Command or Code

Attempt to install cordova@11 using node 12.

Environment, Platform, Device

Node 12.

Version information

Cordova 11.1

Checklist

• I searched for existing GitHub issues
• I updated all Cordova tooling to most recent version
• I included all the necessary information above

From @raphinesse on July 5, 2018 8:23

Current situation

Arguments

/**
 * Usage:
 * @dir - directory where the project will be created. Required.
 * @optionalId - app id. Required (but be "undefined")
 * @optionalName - app name. Required (but can be "undefined").
 * @cfg - extra config to be saved in .cordova/config.json Required (but can be "{}").
 * @extEvents - An EventEmitter instance that will be used for logging purposes. Required (but can be "undefined").
 **/
// Returns a promise.
function (dir, optionalId, optionalName, cfg, extEvents) {...}

Properties of cfg used in cordovaCreate¹

{
    lib: {
        www: {
            // The path/url/npm-name to the template that should be used
            url: String,

            // Symlink instead of copy files from template to dir
            link: Boolean,

            // Template is only fetched when true.
            // Template files are only copied when true.
            // If false, only some "mandatory" files are copied over from
            // `cordova-app-hello-world`
            template: Boolean,

            // Deprecated alias for url (w/out deprecation warning)
            uri: String
        }
    }
}

¹: neither the cfg object nor parts larger than single leaf properties are
passed outside the scope of this module, so a local view on this is all we need.

Pain Points

• Required but optional™ arguments
• Deeply nested structure of configuration
• Confusing naming and unclear semantics of configuration

Proposal

Arguments

/**
 * Creates a new cordova project in dest.
 *
 * @param {string} dest - directory where the project will be created.
 * @param {Object} [opts={}] - options to be used for creating a new cordova project.
 * @returns {Promise} Promise that resolves when project creation has finished
 */
function (dest, opts) {...}

Structure of opts

{
    // Attributes to be set in package.json & config.xml
    id: String,
    name: String,
    version: String,

    // The path/url/npm-name to the template that should be used
    template: String,

    // Symlink instead of copy files from template to dest
    link: Boolean,

    // An EventEmitter instance that will be used for logging purposes
    // If not dropped as proposed in §4
    extEvents: EventEmitter
}

Migrated from apache/cordova-discuss#89

Copied from original issue: apache/cordova-discuss#99

Environment

Tested on Ubuntu (18.04.1), but the same issue is occurring on Mac.
Cordova version 8.1.2 ([email protected])

Steps to reproduce

mkdir build@2
cd build@2
npm install cordova
node_modules/.bin/cordova create hello com.example.hello HelloWorld

What happens

Creating a new cordova project.
Unhandled "error" event. (  Error from Cordova Fetch: Error: npm: Command failed with exit code 1 Error output:
npm ERR! code ENOLOCAL
npm ERR! Could not install "../build@2/node_modules/cordova/node_modules/cordova-app-hello-world/index.js" as it is not a directory and is not a file with a name ending in .tgz, .tar.gz or .tar

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2018-12-05T15_21_50_141Z-debug.log)

Why this happens

1. Cordova create wants to use the default template at ../build@2/node_modules/cordova/node_modules/cordova-app-hello-world/index.js
2. It uses isRemoteUri() [source] to determine whether the template location is a local path or remote URI. This notices the @ and decides it's a remote URI.
3. Because the tempalte location is detected as remote, Cordova-create uses NPM to try and install it [source].
4. NPM chokes on the path as it isn't a directory or tarball.

Why this matters

We ran into this while automatically building Cordova projects using Jenkins.

• We're using a local installation of Cordova (not npm add --global) so we can easily pin different versions for each separate project and keep project configuration local to each Jenkins workspace.
• Jenkins uses @ to split workspaces when running multiple concurrent builds. See hudson.slaves.WorkspaceList under https://wiki.jenkins.io/display/JENKINS/Features+controlled+by+system+properties.

We can work around the problem by reconfiguring Jenkins, but it feels like others will probably run into this problem. Is there a reason why paths with @ are automatically assumed to be remote, even if they exist on the filesystem?

From @raphinesse on July 5, 2018 8:40

Current situation

Right at the start, cordovaCreate calls the following function with the
extEvents argument passed to it.

/**
 * Sets up to forward events to another instance, or log console.
 * This will make the create internal events visible outside
 * @param  {EventEmitter} externalEventEmitter An EventEmitter instance that will be used for
 *   logging purposes. If no EventEmitter provided, all events will be logged to console
 * @return {EventEmitter}
 */
function setupEvents (externalEventEmitter) {
    if (externalEventEmitter) {
        // This will make the platform internal events visible outside
        events.forwardEventsTo(externalEventEmitter);
    // There is no logger if external emitter is not present,
    // so attach a console logger
    } else {
        CordovaLogger.subscribe(events);
    }
    return events;
}

Pain Points

• Every call to cordovaCreate w/out extEvents subscribes CordovaLogger to
  events again w/ no possibility to unsubscribe it.
• The forwarding we setup isn't scoped to cordovaCreate either.
• cordovaCreate is tightly coupled to the Cordova event bus singleton.
• We cannot even rely on the Cordova event bus to be a singleton. Modules can get different versions of cordova-common (e.g. during development with linked dependencies). Then we have multiple event buses with no sane way of choosing which one we want emit events on.

Proposal

The only sane way I see is to accept an EventEmitter as an option (just as we do now), and only emit events there:

const emit = extEvents
    ? (...args) => extEvents.emit(...args)
    : () => {};

This would solve all problems described above.

We should also consider applying this pattern to other Cordova components.

Evolved from apache/cordova-discuss#89

Copied from original issue: apache/cordova-discuss#101