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

meta: ignore require-default-props lint rule for function components #5253

Merged
merged 5 commits into from
Jun 17, 2024

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jun 17, 2024

Follow up of: #5222

In the end, we'd rather trust TS alone to tell us when we forgot to set a default value for a prop that shouldn't be undefined.

Copy link
Contributor

github-actions bot commented Jun 17, 2024

Diff output files
No diff

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

All the explicit = undefined seem silly as that's already the default?

@aduh95
Copy link
Contributor Author

aduh95 commented Jun 17, 2024

All the explicit = undefined seem silly as that's already the default?

I don't know about silliness, comments also do not change runtime behavior, but you wouldn't argue they're silly, would you? It communicates to the reader that this is an optional prop (and the default value is undefined)

@Murderlon
Copy link
Member

But we have TS exactly for that, to indicate what is optional and what isn't? I'm not sure we want to make a habit to type every single optional property in arguments to = undefined.

@aduh95 aduh95 changed the title do not disable require-default-props lint rule meta: ignore require-default-props lint rule for function components Jun 17, 2024
@aduh95 aduh95 requested a review from Murderlon June 17, 2024 15:51
@aduh95 aduh95 merged commit 287764f into transloadit:4.x Jun 17, 2024
17 checks passed
@aduh95 aduh95 deleted the require-default-props branch June 17, 2024 18:15
github-actions bot added a commit that referenced this pull request Jun 19, 2024
| Package              |       Version | Package              |       Version |
| -------------------- | ------------- | -------------------- | ------------- |
| @uppy/aws-s3         |  4.0.0-beta.7 | @uppy/locales        |  4.0.0-beta.4 |
| @uppy/box            |  3.0.0-beta.7 | @uppy/onedrive       |  4.0.0-beta.7 |
| @uppy/companion      | 5.0.0-beta.10 | @uppy/provider-views |  4.0.0-beta.9 |
| @uppy/core           | 4.0.0-beta.10 | @uppy/react          |  4.0.0-beta.7 |
| @uppy/dashboard      | 4.0.0-beta.10 | @uppy/remote-sources |  2.0.0-beta.5 |
| @uppy/dropbox        |  4.0.0-beta.8 | @uppy/transloadit    |  4.0.0-beta.9 |
| @uppy/google-drive   |  3.6.0-beta.1 | uppy                 | 4.0.0-beta.12 |
| @uppy/google-photos  |  0.2.0-beta.1 |                      |               |

- meta: ignore `require-default-props` lint rule for function components (Antoine du Hamel / #5253)
- @uppy/provider-views: Renames & `eslint-disable react/require-default-props` removal (Evgenia Karunus / #5251)
- @uppy/companion: coalesce options `bucket` and `getKey` (Mikael Finstad / #5169)
- @uppy/aws-s3: add `endpoint` option (Antoine du Hamel / #5173)
- @uppy/locales: fix `fa_IR` export (Merlijn Vos / #5241)
- @uppy/companion: improve companion logging (Mikael Finstad / #5250)
- @uppy/transloadit: also fix outdated assembly transloadit:result (Merlijn Vos / #5246)
- docs: fix typo in the url (Evgenia Karunus)
- examples,@uppy/locales,@uppy/provider-views,@uppy/transloadit: Release: [email protected] (github-actions[bot] / #5242)
- meta: Improve aws-node example readme (Artur Paikin / #4753)
- @uppy/locales: Added translation string (it_IT) (Samuel / #5237)
- @uppy/transloadit: fix transloadit:result event (Merlijn Vos / #5231)
- @uppy/provider-views: fix wrong font for files (Merlijn Vos / #5234)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants