search

EddyVerbruggen / Custom-URL-scheme

:link: Launch your Cordova/PhoneGap app by a Custom URL scheme like mycoolapp://
1.03k stars 365 forks source link

Replace requireCordovaModule() with require() #307

Closed davidofwatkins closed 4 years ago

davidofwatkins commented 4 years ago

Cordova 9 throws an error when plugins use requireCordovaModule() for packages that aren't Cordova-related (see https://github.com/apache/cordova-cli/issues/416). This addresses that error in a few places used by Windows.

andreyqin commented 4 years ago

@EddyVerbruggen Hi! Can we have this merged and release a new version with this fix included? This issue prevents us from updating to Cordova 9.

EddyVerbruggen commented 4 years ago

OK, thanks for asking. I'll do a major version bump in a minute.

andreyqin commented 4 years ago

@EddyVerbruggen thank you very much, really appreciate it!

andreyqin commented 4 years ago

@davidofwatkins it looks like you forgot to replace one more requireCordovaModule in prepare-manifest.js. Was there a reason for that?

@EddyVerbruggen should I create a separate PR to fix that?

davidofwatkins commented 4 years ago

@andreyqin Yes, that requireCordovaModule is actually requiring a Cordova module, which as far as I understand is okay:

https://github.com/EddyVerbruggen/Custom-URL-scheme/blob/cd7cc4552cab4f82c991fd7725023de5c5b5ac48/src/windows/hooks/prepare-manifest.js#L5

Is it causing trouble for you?

andreyqin commented 4 years ago

@davidofwatkins seems like you're right. Cordova has deprecated this method only for non-cordova modules and left it available for cordova-* dependencies (https://github.com/apache/cordova-lib/issues/689). I'll let you know if I experience any trouble with it later. Thank you!