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.

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

(improvement) Do not require the www-folder to detect a Cordova-based project. #155

Open
Domvel opened this issue Sep 19, 2019 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Domvel
Copy link

Domvel commented Sep 19, 2019

Improvement (Bug? Feature Request?)

Currently Cordova requires the file config.xml and the www folder to detect a Cordova-based project.

The need of the www-folder is not necessary. It does not matter if it is empty or not. And Cordova will not touch it. (e.g. for installing a native plugin). Cordova is just happy if the www-folder is there.

It's annoying if the www-folder is the dist (build) folder and not tracked by git.
In this case I have to add a .gitkeep file and ignore all other files inside for git.
To always keep it.

If you try to install a plugin without the www-folder, you'll get the error:

cordova plugin add cordova-plugin-inappbrowser --verbose
Current working directory is not a Cordova-based project.

Expected behavior:
Only detect the Cordova-based project by the config.xml.

Q: Why? The www-folder is the source Folder for Cordova!

A: Well, it is used to build the apk. But If you use frameworks like Ionic / Angular you have another source base (src-folder) and build to the www-folder to finally pack it to the apk with Cordova.
And the www-folder is not a part of the git-repository.

What do you think? Can we do without the www-folder? It's just a dummy in this case.

Cordvoa Version 9.0.0

Workaround

.gitignore

www/*
!www/.gitkeep

Add a .gitkeep file to the www-folder.

@Domvel Domvel changed the title (improvement) Do not require the www-folder for e.g. adding plugins. (improvement) Do not require the www-folder to detect a Cordvoa-based project. Sep 19, 2019
@Domvel Domvel changed the title (improvement) Do not require the www-folder to detect a Cordvoa-based project. (improvement) Do not require the www-folder to detect a Cordova-based project. Sep 19, 2019
@brodycj brodycj added enhancement New feature or request help wanted Extra attention is needed labels Dec 30, 2019
@brodycj
Copy link

brodycj commented Dec 30, 2019

I would definitely favor this kind of an enhancement. Unfortunately I don't have much time to work on this right now, and other maintainers seem to be pretty overloaded as well.

IIRC this would be done in cordova-lib. I think it should check for some things such as package.json and config.xml with proper Cordova configuration.

@raphinesse
Copy link
Contributor

Copy of my comment from apache/cordova-cli#529 for better visibility:

I think not having to rely on the presence of the www folder is a reasonable request. But before that change can be implemented sanely, we have to DRY our implementations of the methods to find the Cordova project root.

Currently we have this implemented similarly (if not identically) in at least:

  • cordova-lib/src/cordova/util.js
  • cordova-common/src/CordovaCheck.js
  • cordova-serve/src/util.js

where the version in cordova-common is not actually used anywhere AFAICT.

@raphinesse
Copy link
Contributor

It is worth to mention that cordova-serve does not yet depend on cordova-common. If we want to keep it that way, we could evaluate if it really needs to search for the project dir, or if it can expect to be passed the proper dir from its caller. We could then also remove the implementation from cordova-common and fix this issue in cordova-lib.

The alternative would be to add a dependency to cordova-common, remove the two other implementations and fix this in cordova-common.

Any opinions on this?

@raphinesse
Copy link
Contributor

Regardless of where we put it, one possibility for the new algorithm could be:

const fs = require('fs');
const path = require('path');
const pkgUpSync = require('pkg-up').sync;
 
module.exports = function findCordovaRootUp (cwd) {
    while (cwd && !fs.existsSync(path.join(cwd, 'config.xml'))) {
       pkgUpSync({ cwd });
    }
    return cwd;
}

I.e. search upward from the given dir until we find a package.json beside a config.xml. Return null if no match.

@erisu
Copy link
Member

erisu commented Sep 24, 2020

In the case of using Cordova Electron platform, these two files do exist side-by-side at the platform level.

E.g.

cordovaTest/platforms/electron/www/config.xml
cordovaTest/platforms/electron/www/package.json

If someone ran a Cordova command within that folder, the findCordovaRootUp would return a false positive.

Also, the package.json file at that directory is not a copy of the Cordova's project package.json. That is the package.json for the Electron app.

It might be possible to verify that the cordova property does not exist as it exists in the Cordova project's pacakge.json.

@raphinesse
Copy link
Contributor

Good point about the electron case, @erisu.

So that means we need a different process after all 😞 Random ideas:

  • look at the content of the files
  • (re-)introduce some .cordova file or folder in the project root...
  • allow users to opt-in to explicitly set the root dir via CLI

A solution could also combine several of the above points

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants