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

fix: allow undefined ref prop #4347

Merged
merged 2 commits into from
May 27, 2023
Merged

Conversation

DustinJSilk
Copy link
Contributor

Overview

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

Allow undefined to be passed into ref props.

Fixes #4346

If this is not how you would approach this problem @manucorporat, let me know! But hopefully this is a quick win.

Use cases and why

Please see the linked issue above for a use case example

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@stackblitz
Copy link

stackblitz bot commented May 26, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@manucorporat
Copy link
Contributor

manucorporat commented May 26, 2023

The PR looks great! but we might need to a test, i think the best would be a e2e test.

In this file, let's add a new component:
https://github.com/BuilderIO/qwik/blob/92333c801847716b6b464569cefdb6ac0cf27262/starters/apps/e2e/src/components/render/render.tsx#L98-L99

then a test here:
https://github.com/BuilderIO/qwik/blob/92333c801847716b6b464569cefdb6ac0cf27262/starters/e2e/e2e.render.spec.ts#L465

Notice, it should be some new component that somehow breaks the current e2e test, but passes with the fix!

@DustinJSilk
Copy link
Contributor Author

Awesome, i've added a test to go along with it

@manucorporat manucorporat merged commit ac0ca78 into QwikDev:main May 27, 2023
19 checks passed
@manucorporat
Copy link
Contributor

Awesome work!!

@renovate renovate bot mentioned this pull request Jul 6, 2023
1 task
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.

[🐞] Can't pass undefined to ref prop
2 participants