search

apache / cordova-browser

Apache Cordova
Apache License 2.0
170 stars 85 forks source link

Cordova requirements crash with "Cannot read property 'forEach' of undefined" #65

Closed danielgomezrico closed 5 years ago

danielgomezrico commented 5 years ago

Version used: 5.0.3 and 5.0.4

After running:

cordova requierements

it prints:

$ cordova requirements -d

Checking opts.platforms  :
PlatformApi successfully found for platform browser
PlatformApi successfully found for platform ios

Requirements check results for browser:
Cannot read property 'forEach' of undefined
TypeError: Cannot read property 'forEach' of undefined
    at /usr/local/lib/node_modules/cordova/src/cli.js:406:35
    at Array.map (<anonymous>)
    at /usr/local/lib/node_modules/cordova/src/cli.js:397:68
    at _fulfilled (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:854:54)
    at /usr/local/lib/node_modules/cordova/node_modules/q/q.js:883:30
    at Promise.promise.promiseDispatch (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:816:13)
    at /usr/local/lib/node_modules/cordova/node_modules/q/q.js:624:44
    at runSingle (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:137:13)
    at flush (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:125:13)
    at process._tickCallback (internal/process/next_tick.js:61:11)

After adding some logs on cordova/clj.js I was able to see an undefined in the platformChecks from https://github.com/apache/cordova-cli/blob/master/src/cli.js#L375:

Debug-Values: browser undefined
Debug-Values: ios [object Object],[object Object],[object Object],[object Object]

Any idea if this is caused by this plugin or by my configuration?

janpio commented 5 years ago

What CLI version are you using? Can you maybe install the CLI from master or nightly and try with this version? I recall there were lots of fixes to the requirements functionality in there.

(In general: The browser platform has no real requirements.)

danielgomezrico commented 5 years ago

@janpio Im using cli 8.1.2 (cordova-lib@8.1.1) but it happens with 7.1.0 too, I will try that 👍

danielgomezrico commented 5 years ago

@janpio tried from master and it prints:

Cannot read property 'version' of undefined
TypeError: Cannot read property 'version' of undefined
    at /usr/local/lib/node_modules/cordova/src/cli.js:391:83
    at Array.forEach (<anonymous>)
    at /usr/local/lib/node_modules/cordova/src/cli.js:388:39
    at Array.map (<anonymous>)
    at /usr/local/lib/node_modules/cordova/src/cli.js:375:68
    at _fulfilled (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:854:54)
    at /usr/local/lib/node_modules/cordova/node_modules/q/q.js:883:30
    at Promise.promise.promiseDispatch (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:816:13)
    at /usr/local/lib/node_modules/cordova/node_modules/q/q.js:624:44
    at runSingle (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:137:13)

But it looks like master is behind 8.1.2 (older than npm repository one)

janpio commented 5 years ago

8.1.2 fixed some stuff that was messed up in 8.1.1 - but I checked and master is indeed identical to 8.1.1/2 - so my advice was just bad.

So yep, bug in here or in the CLI that someone has to debug and fix.

danielgomezrico commented 5 years ago

@janpio here's an example https://github.com/caipivara/test-cordova-browser-failure

You can run:

cordova requirements

Just there to see the error happening

brodybits commented 5 years ago

https://github.com/caipivara/test-cordova-browser-failure

I see the bug with Cordova CLI 8.1.2.

If I try with cordova@nightly (nightly build from master branch of cordova-cli) I see the following:

this.isCordova is not a function
TypeError: this.isCordova is not a function
    at preProcessOptions (/Users/brodybits/.nvs/node/11.2.0/x64/lib/node_modules/cordova/node_modules/cordova-lib/src/cordova/util.js:292:28)
    at Promise.resolve.then (/Users/brodybits/.nvs/node/11.2.0/x64/lib/node_modules/cordova/node_modules/cordova-lib/src/cordova/requirements.js:36:37)
    at process._tickCallback (internal/process/next_tick.js:43:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:778:11)
    at startup (internal/bootstrap/node.js:300:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:826:3)

(I use https://github.com/jasongin/nvs to switch between Node.js versions, with cordova@nightly installed with Node.js 11.)

@caipivara I highly recommend you do not commit node_modules, plugins, or platforms unless absolutely necessary. Each developer on a Cordova project can regenerate using cordova prepare.

Thanks for reporting.

danielgomezrico commented 5 years ago

@caipivara I highly recommend you do not commit node_modules, plugins, or platforms unless absolutely necessary. Each developer on a Cordova project can regenerate using cordova prepare.

👍 got it thanks

janpio commented 5 years ago

(Which reminds me we should just make sure that a .gitignore is generated for new projects: https://github.com/apache/cordova-app-hello-world/issues/30)

brodybits commented 5 years ago

If I would do npm install in my checkout of cordova-cli master branch it seems to work for me. It seems to be already fixed in apache/cordova-cli@f10a3719dce40f165f00864110ea6bcae336b38e (references https://issues.apache.org/jira/browse/CB-13740), so we should be able to port it to 8.1.x.

I would also like to see a unit test that verifies that this issue does not reappear in the future.

brodybits commented 5 years ago

I just raised https://github.com/apache/cordova-cli/pull/366 with a proposal to release the solution in apache/cordova-cli#335 in a patch release. Too bad we missed it when we made the cordova-cli patch release last October.

I cannot promise when I or anyone else will have a chance to make the new cordova-cli release.

janpio commented 5 years ago

I repeat:

(In general: The browser platform has no real requirements.)

So this really is not a pressing concern.

brodybits commented 5 years ago

If someone would do cordova requirements on a project with multiple platforms including browser and iOS, it would stop with the error message on browser and miss iOS. I would classify this as a UX issue in general, too bad we missed apache/cordova-cli#335 when we made the patch release in October.

philomorphism commented 5 years ago

this error comes only if we've added browser to platforms!! all other objects(androind, ios, windows etc.) of platforms are working properly!!

i think the source code should be edited in next version of cordova, to ignore this "message"

because as @janpio said there is no "actual" requirements for browser platform since our code itself is written in brower-native languages like html-css-js!!

erisu commented 5 years ago

This issue has been resolved in https://github.com/apache/cordova-browser/pull/61 and in browser@6.0.0.

pashog commented 4 years ago

@janpio tried from master and it prints:

Cannot read property 'version' of undefined
TypeError: Cannot read property 'version' of undefined
    at /usr/local/lib/node_modules/cordova/src/cli.js:391:83
    at Array.forEach (<anonymous>)
    at /usr/local/lib/node_modules/cordova/src/cli.js:388:39
    at Array.map (<anonymous>)
    at /usr/local/lib/node_modules/cordova/src/cli.js:375:68
    at _fulfilled (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:854:54)
    at /usr/local/lib/node_modules/cordova/node_modules/q/q.js:883:30
    at Promise.promise.promiseDispatch (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:816:13)
    at /usr/local/lib/node_modules/cordova/node_modules/q/q.js:624:44
    at runSingle (/usr/local/lib/node_modules/cordova/node_modules/q/q.js:137:13)

But it looks like master is behind 8.1.2 (older than npm repository one)

Have the same error with 9.0.0 (cordova-lib@9.0.1). In my case, an error appeared after removing all the platforms and adding an old android@5.1