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

Convert blob package to TS #62569

Merged
merged 7 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions packages/blob/src/index.js → packages/blob/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/**
* @type {Record<string, File|undefined>}
*/
const cache = {};
type BlobPart = BufferSource | Blob | string;

const cache: Record< string, File > = {};
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't need this, it's part of the DOM types.

Suggested change
type BlobPart = BufferSource | Blob | string;


/**
* Create a blob URL from a file.
Expand All @@ -10,7 +9,7 @@ const cache = {};
*
* @return {string} The blob URL.
*/
export function createBlobURL( file ) {
export function createBlobURL( file: File ): string {
const url = window.URL.createObjectURL( file );

cache[ url ] = file;
Expand All @@ -27,7 +26,7 @@ export function createBlobURL( file ) {
*
* @return {File|undefined} The file for the blob URL.
*/
export function getBlobByURL( url ) {
export function getBlobByURL( url: string ): File | undefined {
return cache[ url ];
}

Expand All @@ -40,7 +39,7 @@ export function getBlobByURL( url ) {
*
* @return {string|undefined} The blob type.
*/
export function getBlobTypeByURL( url ) {
export function getBlobTypeByURL( url: string ): string | undefined {
return getBlobByURL( url )?.type.split( '/' )[ 0 ]; // 0: media type , 1: file extension eg ( type: 'image/jpeg' ).
}

Expand All @@ -49,7 +48,7 @@ export function getBlobTypeByURL( url ) {
*
* @param {string} url The blob URL.
*/
export function revokeBlobURL( url ) {
export function revokeBlobURL( url: string ): void {
if ( cache[ url ] ) {
window.URL.revokeObjectURL( url );
}
Expand All @@ -64,7 +63,7 @@ export function revokeBlobURL( url ) {
*
* @return {boolean} Is the url a blob url?
*/
export function isBlobURL( url ) {
export function isBlobURL( url: string | undefined ): boolean {
if ( ! url || ! url.indexOf ) {
return false;
}
Expand Down Expand Up @@ -94,7 +93,11 @@ export function isBlobURL( url ) {
* @param {BlobPart} content File content (BufferSource | Blob | string).
* @param {string} contentType (Optional) File mime type. Default is `''`.
Copy link
Member

Choose a reason for hiding this comment

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

Eslint complained here about the param type, but this has been converted to TypeScript so we need to remove all the types from JSDoc.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sirreal please review now I have removed the param type.

*/
export function downloadBlob( filename, content, contentType = '' ) {
export function downloadBlob(
filename: string,
content: BlobPart,
contentType: string = ''
): void {
if ( ! filename || ! content ) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* Internal dependencies
*/
import { isBlobURL, getBlobTypeByURL, downloadBlob } from '../';
import { isBlobURL, getBlobTypeByURL, downloadBlob } from '..';

describe( 'isBlobURL', () => {
it( 'returns true if the url starts with "blob:"', () => {
Expand All @@ -13,6 +13,8 @@ describe( 'isBlobURL', () => {
} );

it( 'returns false if the url is not defined', () => {
// calling isBlobURL without a URL is not type compliant, so ignore it
// @ts-ignore

This comment was marked as resolved.

expect( isBlobURL() ).toBe( false );
} );
} );
Expand All @@ -23,6 +25,8 @@ describe( 'getBlobTypeByURL', () => {
} );

it( 'returns undefined if the url is not defined', () => {
// calling getBlobTypeByURL without a URL is not type compliant, so ignore it
// @ts-ignore
expect( getBlobTypeByURL() ).toBe( undefined );

This comment was marked as resolved.

} );
} );
Expand All @@ -36,17 +40,18 @@ describe( 'downloadBlob', () => {
const createElementSpy = jest
.spyOn( global.document, 'createElement' )
.mockReturnValue( mockAnchorElement );

const mockBlob = jest.fn();
const blobSpy = jest.spyOn( window, 'Blob' ).mockReturnValue( mockBlob );
const blobSpy = jest
.spyOn( window, 'Blob' )
.mockReturnValue( mockBlob as unknown as Blob );
jest.spyOn( document.body, 'appendChild' );
jest.spyOn( document.body, 'removeChild' );
beforeEach( () => {
// Can't seem to spy on these static methods. They are `undefined`.
// Possibly overwritten: https://github.com/WordPress/gutenberg/blob/trunk/packages/jest-preset-default/scripts/setup-globals.js#L5
window.URL = {
createObjectURL,
revokeObjectURL,
};
window.URL.createObjectURL = createObjectURL;
window.URL.revokeObjectURL = revokeObjectURL;

This comment was marked as resolved.

} );

afterAll( () => {
Expand Down
Loading