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

@uppy/react: refactor to ESM #3780

Merged
merged 3 commits into from May 25, 2022
Merged

@uppy/react: refactor to ESM #3780

merged 3 commits into from May 25, 2022

Conversation

aduh95
Copy link
Member

@aduh95 aduh95 commented May 24, 2022

No description provided.

@aduh95 aduh95 marked this pull request as ready for review May 24, 2022 14:10
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 24, 2022
@github-actions github-actions bot removed the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 24, 2022
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label May 24, 2022
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels May 24, 2022
@aduh95 aduh95 added 3.0 safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels May 24, 2022
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels May 24, 2022
@@ -53,7 +53,7 @@ export default class Dashboard extends UIPlugin {

this.defaultLocale = locale

// set default options
// set default options, must be kept in sync with packages/@uppy/react/src/DashboardModal.js
Copy link
Member

Choose a reason for hiding this comment

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

As we import Dashboard anyway, couldn't we export and use the default options?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the linter wants them in the same file :/

Copy link
Member

Choose a reason for hiding this comment

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

The linter is here to serve us, not the other way around. If we feel it's more maintainable to not having to remember to keep options in sync across the repo — we should slap an eslint-disable on it and reuse the settings :)

Copy link
Member Author

Choose a reason for hiding this comment

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

If we want to ignore the warning, we don’t need default props at all (I think) we can simply go back to how it was before that and pass the props along using …this.props. There could be good reasons not to do that though, I suppose the lint rule is there for a reason.

Copy link
Member

Choose a reason for hiding this comment

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

I read it wrong, I thought we had the same defaultOptions object in both Dashboard and react/Dashboard. But the latter has prop types instead of the same object. Probably makes most sense yeah

thumbnailWidth: PropTypes.number,
locale,
}
// Must be kept in sync with @uppy/dashboard/src/Dashboard.jsx.
Copy link
Member

Choose a reason for hiding this comment

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

Kudos for writing all these prop types. But yeah, this is why you want TS, this would be reused

@aduh95 aduh95 merged commit d7c29c8 into transloadit:3.x May 25, 2022
@aduh95 aduh95 deleted the react-esm branch May 25, 2022 10:20
aduh95 added a commit that referenced this pull request May 30, 2022
aduh95 added a commit that referenced this pull request May 30, 2022
aduh95 added a commit that referenced this pull request May 30, 2022
github-actions bot added a commit that referenced this pull request May 30, 2022
| Package                   |    Version | Package                   |    Version |
| ------------------------- | ---------- | ------------------------- | ---------- |
| @uppy/audio               | 3.0.0-beta | @uppy/progress-bar        | 3.0.0-beta |
| @uppy/aws-s3              | 3.0.0-beta | @uppy/provider-views      | 3.0.0-beta |
| @uppy/aws-s3-multipart    | 3.0.0-beta | @uppy/react               | 3.0.0-beta |
| @uppy/box                 | 3.0.0-beta | @uppy/redux-dev-tools     | 3.0.0-beta |
| @uppy/companion           | 4.0.0-beta | @uppy/robodog             | 3.0.0-beta |
| @uppy/companion-client    | 3.0.0-beta | @uppy/screen-capture      | 3.0.0-beta |
| @uppy/compressor          | 3.0.0-beta | @uppy/status-bar          | 3.0.0-beta |
| @uppy/core                | 3.0.0-beta | @uppy/store-default       | 3.0.0-beta |
| @uppy/dashboard           | 3.0.0-beta | @uppy/store-redux         | 3.0.0-beta |
| @uppy/drag-drop           | 3.0.0-beta | @uppy/svelte              | 3.0.0-beta |
| @uppy/drop-target         | 3.0.0-beta | @uppy/thumbnail-generator | 3.0.0-beta |
| @uppy/dropbox             | 3.0.0-beta | @uppy/transloadit         | 3.0.0-beta |
| @uppy/facebook            | 3.0.0-beta | @uppy/tus                 | 3.0.0-beta |
| @uppy/file-input          | 3.0.0-beta | @uppy/unsplash            | 3.0.0-beta |
| @uppy/form                | 3.0.0-beta | @uppy/url                 | 3.0.0-beta |
| @uppy/golden-retriever    | 3.0.0-beta | @uppy/utils               | 5.0.0-beta |
| @uppy/google-drive        | 3.0.0-beta | @uppy/vue                 | 3.0.0-beta |
| @uppy/image-editor        | 3.0.0-beta | @uppy/webcam              | 3.0.0-beta |
| @uppy/informer            | 3.0.0-beta | @uppy/xhr-upload          | 3.0.0-beta |
| @uppy/instagram           | 3.0.0-beta | @uppy/zoom                | 3.0.0-beta |
| @uppy/locales             | 3.0.0-beta | uppy                      | 3.0.0-beta |
| @uppy/onedrive            | 3.0.0-beta |                           |            |

- meta: temporary adjust release script for the beta (Antoine du Hamel)
- meta: disable ESM to CJS transform in dist files (Antoine du Hamel / #3773)
- @uppy/companion: remove `searchProviders` wrapper & move `s3` options (Merlijn Vos / #3781)
- meta: do not test on EOL versions of Node.js (Antoine du Hamel / #3786)
- @uppy/companion: remove support for EOL versions of Node.js (Antoine du Hamel / #3784)
- @uppy/react: refactor to ESM (Antoine du Hamel / #3780)
- @uppy/transloadit: remove IE 10 hack (Antoine du Hamel / #3777)
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package                   |    Version | Package                   |    Version |
| ------------------------- | ---------- | ------------------------- | ---------- |
| @uppy/audio               | 3.0.0-beta | @uppy/progress-bar        | 3.0.0-beta |
| @uppy/aws-s3              | 3.0.0-beta | @uppy/provider-views      | 3.0.0-beta |
| @uppy/aws-s3-multipart    | 3.0.0-beta | @uppy/react               | 3.0.0-beta |
| @uppy/box                 | 3.0.0-beta | @uppy/redux-dev-tools     | 3.0.0-beta |
| @uppy/companion           | 4.0.0-beta | @uppy/robodog             | 3.0.0-beta |
| @uppy/companion-client    | 3.0.0-beta | @uppy/screen-capture      | 3.0.0-beta |
| @uppy/compressor          | 3.0.0-beta | @uppy/status-bar          | 3.0.0-beta |
| @uppy/core                | 3.0.0-beta | @uppy/store-default       | 3.0.0-beta |
| @uppy/dashboard           | 3.0.0-beta | @uppy/store-redux         | 3.0.0-beta |
| @uppy/drag-drop           | 3.0.0-beta | @uppy/svelte              | 3.0.0-beta |
| @uppy/drop-target         | 3.0.0-beta | @uppy/thumbnail-generator | 3.0.0-beta |
| @uppy/dropbox             | 3.0.0-beta | @uppy/transloadit         | 3.0.0-beta |
| @uppy/facebook            | 3.0.0-beta | @uppy/tus                 | 3.0.0-beta |
| @uppy/file-input          | 3.0.0-beta | @uppy/unsplash            | 3.0.0-beta |
| @uppy/form                | 3.0.0-beta | @uppy/url                 | 3.0.0-beta |
| @uppy/golden-retriever    | 3.0.0-beta | @uppy/utils               | 5.0.0-beta |
| @uppy/google-drive        | 3.0.0-beta | @uppy/vue                 | 3.0.0-beta |
| @uppy/image-editor        | 3.0.0-beta | @uppy/webcam              | 3.0.0-beta |
| @uppy/informer            | 3.0.0-beta | @uppy/xhr-upload          | 3.0.0-beta |
| @uppy/instagram           | 3.0.0-beta | @uppy/zoom                | 3.0.0-beta |
| @uppy/locales             | 3.0.0-beta | uppy                      | 3.0.0-beta |
| @uppy/onedrive            | 3.0.0-beta |                           |            |

- meta: temporary adjust release script for the beta (Antoine du Hamel)
- meta: disable ESM to CJS transform in dist files (Antoine du Hamel / transloadit#3773)
- @uppy/companion: remove `searchProviders` wrapper & move `s3` options (Merlijn Vos / transloadit#3781)
- meta: do not test on EOL versions of Node.js (Antoine du Hamel / transloadit#3786)
- @uppy/companion: remove support for EOL versions of Node.js (Antoine du Hamel / transloadit#3784)
- @uppy/react: refactor to ESM (Antoine du Hamel / transloadit#3780)
- @uppy/transloadit: remove IE 10 hack (Antoine du Hamel / transloadit#3777)
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.

None yet

2 participants