-
Notifications
You must be signed in to change notification settings - Fork 114
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
Upgrade the "serve" deployment also to the latest #954
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Related issue: e-mission/e-mission-docs#838 Related PR for cordovabuild: e-mission#952 Related PR for devapp: e-mission/e-mission-devapp#18 Main changes: - need to add "Ventura" to the list of releases to fix: ``` /Users/kshankar/e-mission/phone-rciti-branch/node_modules/macos-release/index.js:27 const [name, version] = nameMap.get(release); ^ TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)) at macosRelease (/Users/kshankar/e-mission/phone-rciti-branch/node_modules/macos-release/index.js:27:26) at osName (/Users/kshankar/e-mission/phone-rciti-branch/node_modules/os-name/index.js:21:18) at new Insight (/Users/kshankar/e-mission/phone-rciti-branch/node_modules/insight/lib/index.js:37:13) at Object.<anonymous> (/Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova/src/telemetry.js:26:15) at Module._compile (node:internal/modules/cjs/loader:1246:14) at Module._extensions..js (node:internal/modules/cjs/loader:1300:10) at Module.load (node:internal/modules/cjs/loader:1103:32) at Module._load (node:internal/modules/cjs/loader:942:12) at Module.require (node:internal/modules/cjs/loader:1127:19) at require (node:internal/modules/helpers:112:18) ``` - we do this by copying over a file that includes Ventura per https://stackoverflow.com/questions/68318289/ionic-fails-building-in-macos-12-monterey - I tried to do this in a more principled fashion by upgrading to the most recent version of cordova - but we are already at the most recent version of cordova, and it still doesn't work - I then tried to compare it to the same file in the cordovabuild version and it was the same ``` $ grep Ventura ~/e-mission/native_code_upgrade/node_modules/macos-release/index.js | wc -l 0 ``` - on further invesigation, it turns out that the issue is not that the more recent version of cordova has a newer version of the macos-release library, but that it doesn't use `Insight` (at least by default), which is the module that calls it - phonegap, which we need for `phonegap serve`, does ``` $ grep -r "new Insight" node_modules/ node_modules//cordova/node_modules/insight/readme.md:const insight = new Insight({ node_modules//cordova/node_modules/insight/readme.md:const insight = new Insight({ node_modules//cordova/node_modules/insight/lib/push.js: const insight = new Insight(message); node_modules//insight/readme.md:const insight = new Insight({ node_modules//insight/readme.md:const insight = new Insight({ node_modules//insight/lib/push.js: const insight = new Insight(msg); node_modules//phonegap/node_modules/cordova/src/telemetry.js:var insight = new Insight({ node_modules//phonegap/lib/cli/analytics.js:var insight = new Insight({ ``` - so there is really no alternative to hot-patching the file - add the cordova `browser` platform to fix: ``` CordovaError: No platforms added to this project. Please use `phonegap platform add <platform>`. at Object.preProcessOptions (/Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/src/cordova/util.js:313:15) at /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/src/cordova/prepare.js:49:40 at process.processTicksAndRejections (node:internal/process/task_queues:95:5) ``` - incidentally, if we don't add the platform, cordova tries to add it itself, and that completely messes up the `node_modules` There are 720 packages ``` $ ls -al node_modules/ | wc -l 720 ``` we try to add the browser, which fails ``` Using cordova-fetch for cordova-browser@^6.0.0 (node:24473) Warning: Accessing non-existent property 'browser' of module exports inside circular dependency (Use `node --trace-warnings ...` to show where the warning was created) Failed to fetch platform cordova-browser@^6.0.0 Probably this is either a connection problem, or platform spec is incorrect. Check your connection and platform name/version/URL. Could not determine package name from output: added 24 packages, changed 1 package, and audited 131 packages in 10s ``` we end up with 111 packages, and `phonegap` is uninstalled as well ``` $ ls -al node_modules | wc -l 111 ``` - and the `phonegap` command is not found either - add shelljs to the dependencies to fix ``` [warning] Unable to load PlatformApi from platform. Error: Cannot find module 'shelljs' [warning] Require stack: [warning] - /Users/kshankar/e-mission/phone-rciti-branch/platforms/browser/cordova/Api.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/src/cordova/util.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/src/platforms/platforms.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/src/plugman/install.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/src/plugman/plugman.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova-lib/cordova-lib.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/node_modules/cordova/cordova.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/lib/cordova/index.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/lib/phonegap/cordova.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/lib/phonegap.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/lib/main.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/lib/cli.js [warning] - /Users/kshankar/e-mission/phone-rciti-branch/node_modules/phonegap/bin/phonegap.js ``` Testing done: ``` $ rm -rf node_modules $ bash setup/setup_serve.sh $ npm run serve ``` the serve process starts up and the devapp is able to connect to it
…into upgrade_serve
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue: e-mission/e-mission-docs#838
Related PR for cordovabuild: #952
Related PR for devapp: e-mission/e-mission-devapp#18
Main changes:
need to add "Ventura" to the list of releases to fix:
we do this by copying over a file that includes Ventura
per https://stackoverflow.com/questions/68318289/ionic-fails-building-in-macos-12-monterey
I tried to do this in a more principled fashion by upgrading to the most recent version of cordova
I then tried to compare it to the same file in the cordovabuild version and it was the same
on further invesigation, it turns out that the issue is not that the more
recent version of cordova has a newer version of the macos-release
library, but that it doesn't use
Insight
(at least by default), whichis the module that calls it
phonegap, which we need for
phonegap serve
, doesso there is really no alternative to hot-patching the file
add the cordova
browser
platform to fix:node_modules
There are 720 packages
we try to add the browser, which fails
we end up with 111 packages, and
phonegap
is uninstalled as wellphonegap
command is not found eitheradd shelljs to the dependencies to fix
Testing done:
the serve process starts up and the devapp is able to connect to it