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.

Sign up for GitHub

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

Jump to bottom

Upgrade the devapp to the most recent version of everything #18

Merged
merged 2 commits into from
Apr 23, 2023
Merged

Upgrade the devapp to the most recent version of everything #18

merged 2 commits into from
Apr 23, 2023

Conversation

shankari
Copy link

To be consistent with 2023 phone upgrades
Related issue: e-mission/e-mission-docs#838
Related PR: e-mission/e-mission-phone#952

Since the rest of the app is not the same, I applied the changes by diffing the
config.xml and package.json and the setup directory with the corresponding
locations in the e-mission-phone repo

Additional minor changes:

  • comment out the configure_xml_and_json step, since we don't have the
    cordovabuild versions in this release
  • change the package name to edu.berkeley.eecs.emission.devapp
  • bump up the versions

Testing done:

  • devapp builds successfully
  • after making phone app changes, devapp displays phone app successfully

The main changes are in a324e18
The package-lock changes are split out into 1a2c8ad since they are messy and hard to review, and are just a representation of the original package.json changes

@shankari
Upgrade the devapp to the most recent version of everything
a324e18 
To be consistent with 2023 phone upgrades
Related issue: e-mission/e-mission-docs#838
Related PR: e-mission/e-mission-phone#952

Since the rest of the app is not the same, I applied the changes by diffing the
config.xml and package.json and the setup directory with the corresponding
locations in the e-mission-phone repo

Additional minor changes:
+ comment out the `configure_xml_and_json` step, since we don't have the
cordovabuild versions in this release
+ change the package name to `edu.berkeley.eecs.emission.devapp`
+ bump up the versions

Testing done:
- devapp builds successfully
- after making phone app changes, devapp displays phone app successfully
@shankari
Update the changes for the package-lock as well
1a2c8ad 
Splitting this from a324e18
since this tends to be super long and messy to review
@shankari shankari merged commit 45fbfc6 into master Apr 23, 2023
shankari added a commit to shankari/e-mission-phone that referenced this pull request Apr 23, 2023
@shankari
Upgrade the "serve" deployment also to the latest
460f6ee 
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
@shankari shankari mentioned this pull request Apr 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@shankari