Skip to content

feat: include option to force cordova inclusion - #8576

Merged
markemer merged 6 commits into
nextfrom
RMET-5379
Aug 20, 2026
Merged

feat: include option to force cordova inclusion#8576
markemer merged 6 commits into
nextfrom
RMET-5379

Conversation

@markemer

@markemer markemer commented Aug 18, 2026

Copy link
Copy Markdown
Member

Description

Change Type

  • Fix
  • Feature
  • Refactor
  • Breaking Change
  • Documentation
  • Other (CI, chores, etc.)

Rationale / Problems Fixed

Tests or Reproductions

Screenshots / Media

Platforms Affected

  • Android
  • iOS
  • Web

Notes / Comments

Signed-off-by: Mark Anderson <mark.anderson@outsystems.com>
@markemer
markemer marked this pull request as ready for review August 18, 2026 18:41
@OS-pedrogustavobilro OS-pedrogustavobilro self-assigned this Aug 20, 2026
Comment thread cli/src/ios/update.ts
Comment thread cli/src/android/update.ts
Comment thread cli/src/android/update.ts Outdated

@OS-pedrogustavobilro OS-pedrogustavobilro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with a few combination of Capacitor apps, everything seems to work fine!

Left an additional non-blocking comment about the logging, but approving the PR.

Comment thread cli/src/android/update.ts
Comment on lines 44 to +48
if (enableCordova) {
logger.info('Found Cordova Plugins: Including Android Cordova Support');
if (config.app.forceCordova) {
logger.info('Cordova support installation has been forced');
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: This if block logs "Found Cordova Plugins" even if there are 0 cordova plugins with config.app.forceCordova set to true, which might be misleading? Unless that's what you were going with.

(also applies to the iOS version of update.ts)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I really have to think about this language/logic a little, but I thought just "warning" (I did wan't to use warn, but I thought people might complain) when it's enabled and forced would be enough for now.

@markemer
markemer merged commit 638fe4d into next Aug 20, 2026
6 checks passed
@markemer
markemer deleted the RMET-5379 branch August 20, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants