cordova-android icon indicating copy to clipboard operation
cordova-android copied to clipboard

fix: remove pollOnce from NativeToJsMessageQueue.popAndEncodeAsJs

Open erisu opened this issue 1 year ago • 3 comments

Motivation and Context

Remove the require block for cordova/plugin/android/polling, a module that doesn't exist.

Resolves #1735

Description

The cordova/plugin/android/polling module appears to have been removed since 2012.

The last known version of Cordova-JS where this module existed was [email protected] - polling.js in the polling.js file, with the version tag created on Sep 13, 2012.

The module was removed in [email protected], a tag/release created on Oct 16, 2012.

The exact commit that removed the module from Cordova-JS does not have an associated PR, GitHub issue, or CB ticket to explain the reason for this change.

Additionally, the platform-specific Cordova-JS modules, which reside in the cordova-js-src directory of this repository (originally labeled as platform_modules), were not created until 2015—2 years and 4 months later. This directory has also never contained the polling module as far as I can see.

Since this part of the code has not been functional since 2012, I believe it is safe to remove it.

Testing

n/a

Checklist

  • [ ] I've run the tests to see all new and existing tests pass
  • [ ] I added automated test coverage as appropriate for this change
  • [ ] Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • [ ] I've updated the documentation if necessary

erisu avatar Nov 13 '24 01:11 erisu

I believe the polling implementation was brought into the main exec file, and still exists here: https://github.com/apache/cordova-android/blob/5a2c50d1ed67c1cdad0d12f1b6fbbd04ab55dff6/cordova-js-src/exec.js#L120-L132

I don't know whether that means we should be calling it from Java or not... clearly this has been broken for a long while

dpogue avatar Nov 13 '24 02:11 dpogue

I believe the polling implementation was brought into the main exec file, and still exists here:

https://github.com/apache/cordova-android/blob/5a2c50d1ed67c1cdad0d12f1b6fbbd04ab55dff6/cordova-js-src/exec.js#L120-L132

I don't know whether that means we should be calling it from Java or not... clearly this has been broken for a long while

Might be able to expose that method by added it to androidExec:

androidExec.pollOnce = pollOnce;

Then instead of deleting the line, update it to:

            if (!willSendAllMessages) {
                sb.append("window.setTimeout(function(){cordova.require('cordova/exec').pollOnce();},0);");
            }

But I don't know what the behavior it will have after adding a pollOnce call to something that never ran for 12 years...

erisu avatar Nov 13 '24 03:11 erisu

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 72.50%. Comparing base (b623311) to head (9a21426). :warning: Report is 46 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1746   +/-   ##
=======================================
  Coverage   72.50%   72.50%           
=======================================
  Files          23       23           
  Lines        1837     1837           
=======================================
  Hits         1332     1332           
  Misses        505      505           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov-commenter avatar Jan 28 '25 03:01 codecov-commenter