Skip to content

refactor(blob): type url parameter as optional#49201

Merged
jorgefilipecosta merged 1 commit intoWordPress:trunkfrom
johnhooks:refactor/blob-url-parameter
Sep 12, 2023
Merged

refactor(blob): type url parameter as optional#49201
jorgefilipecosta merged 1 commit intoWordPress:trunkfrom
johnhooks:refactor/blob-url-parameter

Conversation

@johnhooks
Copy link
Copy Markdown
Contributor

@johnhooks johnhooks commented Mar 20, 2023

What?

Type the url parameter of @wordpress/blob function isBlobUrl as possibly undefined.

Why?

In common usage, the url parameter of the @wordpress/blob functions could be undefined. Though it is not typed as such, causing type errors when passed possibly undefined values in TypeScript.

Example of passing url as possibly undefined in core/audio:

useEffect( () => {
if ( ! id && isBlobURL( src ) ) {
const file = getBlobByURL( src );
if ( file ) {
mediaUpload( {
filesList: [ file ],
onFileChange: ( [ media ] ) => onSelectAudio( media ),
onError: ( e ) => onUploadError( e ),
allowedTypes: ALLOWED_MEDIA_TYPES,
} );
}
}
}, [] );

How?

Type the url parameter as string|undefined. I choose this over making it optional, as in string=, because passing no value isn't how the functions are used.

Testing Instructions

The current tests should be sufficient.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Mar 20, 2023
@github-actions
Copy link
Copy Markdown

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @johnhooks! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@johnhooks johnhooks force-pushed the refactor/blob-url-parameter branch from 6d85b8a to 24fa8be Compare March 25, 2023 01:46
@johnhooks johnhooks force-pushed the refactor/blob-url-parameter branch from 24fa8be to 43cbdd3 Compare March 25, 2023 01:47
@johnhooks johnhooks marked this pull request as ready for review March 25, 2023 11:32
@skorasaurus skorasaurus added the [Package] Blob /packages/blob label Mar 26, 2023
Copy link
Copy Markdown
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

The change looks good to me. The code of isBlobURL supports undefined, and in normal usage undefined can be passed so it seems the typescript types should reflect that.

@jorgefilipecosta jorgefilipecosta merged commit 045cb0d into WordPress:trunk Sep 12, 2023
@github-actions github-actions bot added this to the Gutenberg 16.7 milestone Sep 12, 2023
@mikachan mikachan added the [Type] Enhancement A suggestion for improvement. label Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Package] Blob /packages/blob [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants