From 6638d040a57dfb25754c364b4e2130dbef59a829 Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Wed, 31 Jan 2024 12:22:25 +0800 Subject: [PATCH] linting and typing improvements [#287] (#337) * Typing improvements [#287] * rename FileApiSource to FileSource * In a few cases we need to use any and biome-ignore. Deferring any restructuring here to js v4. * replace prettier with biome --- .github/workflows/actions.yml | 2 +- app/src/MaplibreMap.tsx | 21 ++++++++------- app/src/Start.tsx | 4 +-- js/adapters.ts | 23 +++++++++++----- js/index.ts | 15 ++++++++--- js/package-lock.json | 49 +++++++++++++++++++---------------- js/package.json | 7 +++-- js/test/v3.test.ts | 4 +-- 8 files changed, 74 insertions(+), 51 deletions(-) diff --git a/.github/workflows/actions.yml b/.github/workflows/actions.yml index f886018..c5d6df6 100644 --- a/.github/workflows/actions.yml +++ b/.github/workflows/actions.yml @@ -45,7 +45,7 @@ jobs: node-version: 18.x - run: python .github/check_examples.py - run: cd js && npm install && npm test - - run: cd js && npm run prettier-check + - run: cd js && npm run biome-check - run: cd python && python -m unittest test/test_* - run: cd cpp && make - run: cd serverless/cloudflare && npm install && npm test diff --git a/app/src/MaplibreMap.tsx b/app/src/MaplibreMap.tsx index 5041118..775e481 100644 --- a/app/src/MaplibreMap.tsx +++ b/app/src/MaplibreMap.tsx @@ -102,6 +102,13 @@ interface LayerVisibility { visible: boolean; } +interface PMTilesMetadata { + name?: string; + type?: string; + tilestats?: unknown; + vector_layers: LayerSpecification[]; +} + const LayersVisibilityController = (props: { layers: LayerVisibility[]; onChange: (layers: LayerVisibility[]) => void; @@ -181,7 +188,7 @@ const LayersVisibilityController = (props: { const rasterStyle = async (file: PMTiles): Promise => { let header = await file.getHeader(); - let metadata = await file.getMetadata(); + let metadata = (await file.getMetadata()) as PMTilesMetadata; let layers: LayerSpecification[] = []; if (metadata.type !== "baselayer") { @@ -222,7 +229,7 @@ const vectorStyle = async ( layersVisibility: LayerVisibility[]; }> => { let header = await file.getHeader(); - let metadata = await file.getMetadata(); + let metadata = (await file.getMetadata()) as PMTilesMetadata; let layers: LayerSpecification[] = []; let baseOpacity = 0.35; @@ -233,14 +240,8 @@ const vectorStyle = async ( var tilestats: any; var vector_layers: LayerSpecification[]; - if (metadata.json) { - let j = JSON.parse(metadata.json); - tilestats = j.tilestats; - vector_layers = j.vector_layers; - } else { - tilestats = metadata.tilestats; - vector_layers = metadata.vector_layers; - } + tilestats = metadata.tilestats; + vector_layers = metadata.vector_layers; if (vector_layers) { for (let [i, layer] of vector_layers.entries()) { diff --git a/app/src/Start.tsx b/app/src/Start.tsx index 4c469f4..9d16b65 100644 --- a/app/src/Start.tsx +++ b/app/src/Start.tsx @@ -1,6 +1,6 @@ import { useState, Dispatch, SetStateAction, useCallback } from "react"; import maplibregl from "maplibre-gl"; -import { PMTiles, FileAPISource } from "../../js/index"; +import { PMTiles, FileSource } from "../../js/index"; import { styled } from "./stitches.config"; import { useDropzone } from "react-dropzone"; @@ -108,7 +108,7 @@ function Start(props: { setFile: Dispatch>; }) { const onDrop = useCallback((acceptedFiles: File[]) => { - props.setFile(new PMTiles(new FileAPISource(acceptedFiles[0]))); + props.setFile(new PMTiles(new FileSource(acceptedFiles[0]))); }, []); const { acceptedFiles, getRootProps, getInputProps } = useDropzone({ diff --git a/js/adapters.ts b/js/adapters.ts index 7cce8c1..fb049f3 100644 --- a/js/adapters.ts +++ b/js/adapters.ts @@ -1,15 +1,26 @@ +// biome-ignore lint: needed for Leaflet + IIFE to work declare const L: any; +// biome-ignore lint: needed for window.URL to disambiguate from cloudflare workers declare const window: any; -declare const document: any; +declare const document: DocumentLike; +import type { Coords } from "leaflet"; import { PMTiles, TileType } from "./index"; -export const leafletRasterLayer = (source: PMTiles, options: any) => { +interface DocumentLike { + // biome-ignore lint: we don't want to bring in the entire document type + createElement: (s: string) => any; +} + +// biome-ignore lint: we don't want to bring in the entire document type +type DoneCallback = (error?: Error, tile?: any) => void; + +export const leafletRasterLayer = (source: PMTiles, options: unknown) => { let loaded = false; let mimeType = ""; const cls = L.GridLayer.extend({ - createTile: (coord: any, done: any) => { - const el: any = document.createElement("img"); + createTile: (coord: Coords, done: DoneCallback) => { + const el = document.createElement("img"); const controller = new AbortController(); const signal = controller.signal; el.cancel = () => { @@ -40,8 +51,8 @@ export const leafletRasterLayer = (source: PMTiles, options: any) => { const blob = new Blob([arr.data], { type: mimeType }); const imageUrl = window.URL.createObjectURL(blob); el.src = imageUrl; - el.cancel = null; - done(null, el); + el.cancel = undefined; + done(undefined, el); } }) .catch((e) => { diff --git a/js/index.ts b/js/index.ts index c14e667..b654e63 100644 --- a/js/index.ts +++ b/js/index.ts @@ -160,11 +160,16 @@ async function defaultDecompress( return buf; } if (compression === Compression.Gzip) { + // biome-ignore lint: needed to detect DecompressionStream in browser+node+cloudflare workers if (typeof (globalThis as any).DecompressionStream === "undefined") { return decompressSync(new Uint8Array(buf)); } - const stream = new Response(buf).body!; + const stream = new Response(buf).body; + if (!stream) { + throw Error("Failed to read response stream"); + } const result: ReadableStream = stream.pipeThrough( + // biome-ignore lint: needed to detect DecompressionStream in browser+node+cloudflare workers new (globalThis as any).DecompressionStream("gzip") ); return new Response(result).arrayBuffer(); @@ -258,7 +263,9 @@ export interface Source { getKey: () => string; } -export class FileAPISource implements Source { +// uses the Browser's File API, which is different from the NodeJS file API. +// see https://developer.mozilla.org/en-US/docs/Web/API/File_API +export class FileSource implements Source { file: File; constructor(file: File) { @@ -930,7 +937,7 @@ export class PMTiles { } } - async getMetadataAttempt(): Promise { + async getMetadataAttempt(): Promise { const header = await this.cache.getHeader(this.source); const resp = await this.source.getBytes( @@ -948,7 +955,7 @@ export class PMTiles { return JSON.parse(dec.decode(decompressed)); } - async getMetadata(): Promise { + async getMetadata(): Promise { try { return await this.getMetadataAttempt(); } catch (e) { diff --git a/js/package-lock.json b/js/package-lock.json index 1010f35..d4ea25f 100644 --- a/js/package-lock.json +++ b/js/package-lock.json @@ -9,13 +9,13 @@ "version": "3.0.0-alpha.0", "license": "BSD-3-Clause", "dependencies": { + "@types/leaflet": "^1.9.8", "fflate": "^0.8.0" }, "devDependencies": { "@biomejs/biome": "^1.5.3", "@types/node": "^18.11.9", "esbuild": "^0.20.0", - "prettier": "^2.8.4", "tsx": "^4.7.0", "typescript": "^4.5.5" } @@ -543,6 +543,19 @@ "node": ">=12" } }, + "node_modules/@types/geojson": { + "version": "7946.0.13", + "resolved": "https://registry.npmjs.org/@types/geojson/-/geojson-7946.0.13.tgz", + "integrity": "sha512-bmrNrgKMOhM3WsafmbGmC+6dsF2Z308vLFsQ3a/bT8X8Sv5clVYpPars/UPq+sAaJP+5OoLAYgwbkS5QEJdLUQ==" + }, + "node_modules/@types/leaflet": { + "version": "1.9.8", + "resolved": "https://registry.npmjs.org/@types/leaflet/-/leaflet-1.9.8.tgz", + "integrity": "sha512-EXdsL4EhoUtGm2GC2ZYtXn+Fzc6pluVgagvo2VC1RHWToLGlTRwVYoDpqS/7QXa01rmDyBjJk3Catpf60VMkwg==", + "dependencies": { + "@types/geojson": "*" + } + }, "node_modules/@types/node": { "version": "18.11.9", "resolved": "https://registry.npmjs.org/@types/node/-/node-18.11.9.tgz", @@ -954,21 +967,6 @@ "url": "https://github.com/privatenumber/get-tsconfig?sponsor=1" } }, - "node_modules/prettier": { - "version": "2.8.4", - "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.8.4.tgz", - "integrity": "sha512-vIS4Rlc2FNh0BySk3Wkd6xmwxB0FpOndW5fisM5H8hsZSxU2VWVB5CWIkIjWvrHjIhxk2g3bfMKM87zNTrZddw==", - "dev": true, - "bin": { - "prettier": "bin-prettier.js" - }, - "engines": { - "node": ">=10.13.0" - }, - "funding": { - "url": "https://github.com/prettier/prettier?sponsor=1" - } - }, "node_modules/resolve-pkg-maps": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/resolve-pkg-maps/-/resolve-pkg-maps-1.0.0.tgz", @@ -1315,6 +1313,19 @@ "dev": true, "optional": true }, + "@types/geojson": { + "version": "7946.0.13", + "resolved": "https://registry.npmjs.org/@types/geojson/-/geojson-7946.0.13.tgz", + "integrity": "sha512-bmrNrgKMOhM3WsafmbGmC+6dsF2Z308vLFsQ3a/bT8X8Sv5clVYpPars/UPq+sAaJP+5OoLAYgwbkS5QEJdLUQ==" + }, + "@types/leaflet": { + "version": "1.9.8", + "resolved": "https://registry.npmjs.org/@types/leaflet/-/leaflet-1.9.8.tgz", + "integrity": "sha512-EXdsL4EhoUtGm2GC2ZYtXn+Fzc6pluVgagvo2VC1RHWToLGlTRwVYoDpqS/7QXa01rmDyBjJk3Catpf60VMkwg==", + "requires": { + "@types/geojson": "*" + } + }, "@types/node": { "version": "18.11.9", "resolved": "https://registry.npmjs.org/@types/node/-/node-18.11.9.tgz", @@ -1522,12 +1533,6 @@ "resolve-pkg-maps": "^1.0.0" } }, - "prettier": { - "version": "2.8.4", - "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.8.4.tgz", - "integrity": "sha512-vIS4Rlc2FNh0BySk3Wkd6xmwxB0FpOndW5fisM5H8hsZSxU2VWVB5CWIkIjWvrHjIhxk2g3bfMKM87zNTrZddw==", - "dev": true - }, "resolve-pkg-maps": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/resolve-pkg-maps/-/resolve-pkg-maps-1.0.0.tgz", diff --git a/js/package.json b/js/package.json index 7af9e34..9c513f5 100644 --- a/js/package.json +++ b/js/package.json @@ -17,9 +17,8 @@ "build": "npm run build-iife && npm run build-esm && npm run build-tsc", "test": "tsx test/index.test.ts", "tsc": "tsc --noEmit --watch", - "prettier": "prettier --write *.ts test/*.ts", - "prettier-check": "prettier --check *.ts test/*.ts", - "biome": "biome check adapters.ts index.ts v2.ts test" + "biome": "biome check adapters.ts index.ts v2.ts test --apply", + "biome-check": "biome check adapters.ts index.ts v2.ts test" }, "homepage": "https://github.com/protomaps/pmtiles", "author": "Brandon Liu", @@ -28,11 +27,11 @@ "@biomejs/biome": "^1.5.3", "@types/node": "^18.11.9", "esbuild": "^0.20.0", - "prettier": "^2.8.4", "tsx": "^4.7.0", "typescript": "^4.5.5" }, "dependencies": { + "@types/leaflet": "^1.9.8", "fflate": "^0.8.0" } } diff --git a/js/test/v3.test.ts b/js/test/v3.test.ts index 3e7bae5..711554b 100644 --- a/js/test/v3.test.ts +++ b/js/test/v3.test.ts @@ -371,7 +371,7 @@ test("pmtiles get metadata", async () => { ); const p = new PMTiles(source); const metadata = await p.getMetadata(); - assert.ok(metadata.name); + assert.ok((metadata as { name: string }).name); }); // echo '{"type":"Polygon","coordinates":[[[0,0],[0,1],[1,0],[0,0]]]}' | ./tippecanoe -zg -o test_fixture_2.pmtiles @@ -383,7 +383,7 @@ test("pmtiles handle retries", async () => { source.etag = "1"; const p = new PMTiles(source); const metadata = await p.getMetadata(); - assert.ok(metadata.name); + assert.ok((metadata as { name: string }).name); source.etag = "2"; source.replaceData("test/data/test_fixture_2.pmtiles"); assert.ok(await p.getZxy(0, 0, 0));