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.

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

bug: @ionic/react@8.2.8 -> IonRadio now rendered without the "value" prop #476

Closed
3 tasks done
mrsamse opened this issue Sep 6, 2024 · 3 comments · Fixed by ionic-team/ionic-framework#29869
Closed
3 tasks done
Labels

Comments

@mrsamse
Copy link

mrsamse commented Sep 6, 2024

Prerequisites

Ionic Framework Version

v8.x

Current Behavior

I upgraded my project today from @ionic/react v8.2.7 to v8.3.0 and noticed that the IonRadio now gets rendered without the "value" prop. This behaviour was introduced with the version 8.2.8:

Version 8.2.7 behaviour:
React code:

<IonRadio value="de"><label>Test</label></IonRadio>

HTML Output:

<ion-radio value="de" class="md radio-checked radio-justify-space-between radio-alignment-center radio-label-placement-start ion-activatable ion-focusable" role="radio" aria-checked="true" tabindex="0"><label>Test</label></ion-radio>

Version 8.2.8 behaviour:
React code:

<IonRadio value="de"><label>Test</label></IonRadio>

HTML Output:

<ion-radio class="md radio-checked radio-justify-space-between radio-alignment-center radio-label-placement-start ion-activatable ion-focusable" role="radio" aria-checked="true" tabindex="0"><label>Test</label></ion-radio>

Expected Behavior

Not sure if this was intentional but for us this is not a minor change but a breaking change, which should be declared in the release overview.

If this was unintentional this should be reverted.

Steps to Reproduce

Already described above

Code Reproduction URL

No link proved as everything is stated in the description

Ionic Info

Ionic:

Ionic CLI : 7.2.0 (/usr/local/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/react 8.2.8

Capacitor:

Capacitor CLI : 6.1.2
@capacitor/android : 6.1.2
@capacitor/core : 6.1.2
@capacitor/ios : 6.1.2

Utility:

cordova-res : not installed globally
native-run : 2.0.1

System:

NodeJS : v21.7.3 (/usr/local/Cellar/node/21.7.3/bin/node)
npm : 10.8.2
OS : macOS Unknown

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Sep 6, 2024
@brandyscarney brandyscarney transferred this issue from ionic-team/ionic-framework Sep 9, 2024
@brandyscarney
Copy link
Member

Thank you for the issue! I have moved this to the stencil-ds-output-targets repository since this is the repository that caused this issue. We have an internal issue to investigate this further.

github-merge-queue bot pushed a commit to ionic-team/ionic-framework that referenced this issue Sep 16, 2024
Issue number: resolves ionic-team/stencil-ds-output-targets#476,
resolves ionic-team/stencil-ds-output-targets#475, resolves #29848

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

In v0.6.0 of the [React output
target](https://www.npmjs.com/package/@stencil/react-output-target), the
implementation was changed to leverage Lit's utility for creating React
components from web components. This introduced some unforseen issues
and breaking changes.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Reverts many of the changes from
#29782 to downgrade
the React output target package to the last stable version (v0.5.3)
- Downgrades the version of Stencil to v4.20.0 (due to
ionic-team/stencil#5983 causing problems with
the downgraded output target)
- Pins these versions and prevents Renovate from attempting to upgrade
until the associated issues are resolved

## Does this introduce a breaking change?

- [ ] Yes
- [X] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build for this version: `8.3.1-dev.11726167750.15400355`

I tested the dev build against the use cases outlined in
ionic-team/stencil-ds-output-targets#475 and
ionic-team/stencil-ds-output-targets#476
github-merge-queue bot pushed a commit to ionic-team/ionic-framework that referenced this issue Sep 16, 2024
Issue number: resolves ionic-team/stencil-ds-output-targets#476,
resolves ionic-team/stencil-ds-output-targets#475, resolves #29848

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

In v0.6.0 of the [React output
target](https://www.npmjs.com/package/@stencil/react-output-target), the
implementation was changed to leverage Lit's utility for creating React
components from web components. This introduced some unforseen issues
and breaking changes.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Reverts many of the changes from
#29782 to downgrade
the React output target package to the last stable version (v0.5.3)
- Downgrades the version of Stencil to v4.20.0 (due to
ionic-team/stencil#5983 causing problems with
the downgraded output target)
- Pins these versions and prevents Renovate from attempting to upgrade
until the associated issues are resolved

## Does this introduce a breaking change?

- [ ] Yes
- [X] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build for this version: `8.3.1-dev.11726167750.15400355`

I tested the dev build against the use cases outlined in
ionic-team/stencil-ds-output-targets#475 and
ionic-team/stencil-ds-output-targets#476
@tanner-reits tanner-reits reopened this Sep 16, 2024
brandyscarney pushed a commit to ionic-team/ionic-framework that referenced this issue Sep 17, 2024
Issue number: resolves ionic-team/stencil-ds-output-targets#476,
resolves ionic-team/stencil-ds-output-targets#475, resolves #29848

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

<!-- Please describe the current behavior that you are modifying. -->

In v0.6.0 of the [React output
target](https://www.npmjs.com/package/@stencil/react-output-target), the
implementation was changed to leverage Lit's utility for creating React
components from web components. This introduced some unforseen issues
and breaking changes.

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Reverts many of the changes from
#29782 to downgrade
the React output target package to the last stable version (v0.5.3)
- Downgrades the version of Stencil to v4.20.0 (due to
ionic-team/stencil#5983 causing problems with
the downgraded output target)
- Pins these versions and prevents Renovate from attempting to upgrade
until the associated issues are resolved

- [ ] Yes
- [X] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build for this version: `8.3.1-dev.11726167750.15400355`

I tested the dev build against the use cases outlined in
ionic-team/stencil-ds-output-targets#475 and
ionic-team/stencil-ds-output-targets#476
@tanner-reits
Copy link
Member

@mrsamse We've reverted the version of the React output target used in Ionic Framework as a part of the v8.2.9 and v8.3.1 releases to resolve this issue. I also updated the output target release notes to call this out as a breaking change.

Thank you for letting us know!

Copy link

ionitron-bot bot commented Oct 17, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of the output targets, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Oct 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants