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

feat: Improve loader type safety #3112

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions docs/developer-guide/using-worker-loaders.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,18 @@ async function loadInParallel(url1, url2) {

## Disabling Worker Loaders

Applications can use the `worker: false` option to disable worker loaders, for instance to simplify debugging of parsing issues:
Applications can use the `core.worker: false` option to disable worker loaders, for instance to simplify debugging of parsing issues:

```typescript
async function loadWithoutWorker(url1) {
const data = await load(url1, DracoLoader, {worker: false});
const data = await load(url1, DracoLoader, {core: {worker: false}});
}
```

## Disabling Reuse of Workers

Applications reuse already created workers by default. To avoid `enlarge memory arrays` error it is really necessary to disable it if you need to load multiple datasets in a sequence.
This functionality can be disabled by `reuseWorkers: false` option:
This functionality can be disabled by `core.reuseWorkers: false` option:

```typescript
async function loadWithoutWorker(url1) {
Expand Down Expand Up @@ -103,7 +103,7 @@ A worker loader starts a separate thread with a javascript bundle that only cont

## Debugging Worker Loaders (Advanced)

Debugging worker loaders is tricky. While it is always possible to specify `options.worker: false` which helps in many situations, there are cases where the worker loader itself must be debugged.
Debugging worker loaders is tricky. While it is always possible to specify `options.core.worker: false` which helps in many situations, there are cases where the worker loader itself must be debugged.

TBA - There is an ambition to provide better support for debugging worker loaders:

Expand Down
2 changes: 1 addition & 1 deletion docs/specifications/category-image.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ The image category support some generic options (specified using `options.image.

### About worker loading

Worker loading is only supported for the `data` and `imagebitmap` formats. Since image worker loading is only available on some browsers (Chrome and Firefox), the `ImageLoader` dynamically determines if worker loading is available. Use `options.worker: false` to disable worker loading of images.
Worker loading is only supported for the `data` and `imagebitmap` formats. Since image worker loading is only available on some browsers (Chrome and Firefox), the `ImageLoader` dynamically determines if worker loading is available. Use `options.core.worker: false` to disable worker loading of images.

## Image API

Expand Down
2 changes: 1 addition & 1 deletion docs/whats-new.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ The 2.0 release brings potentially dramatic bundle size savings through dynamic
- **Worker Loaders**

- Ease-of-use: Worker loading is provided by the main loader objects. It is not necessary to import the `...WorkerLoader` objects to enable worker loading (but see below about bundle size)
- Performance: Loading on worker threads is now the default: All worker enabled loaders now run on worker threads by default (set `options.worker: false` to disable worker-thread loading and run the loader on the main thread).
- Performance: Loading on worker threads is now the default: All worker enabled loaders now run on worker threads by default (set `options.core.worker: false` to disable worker-thread loading and run the loader on the main thread).
- Debugging: Development builds of workers are now available on `unpkg.com` CDN, eabling debugging of worker loaders.
- Bundle size: Workers are no longer bundled, but loaded from from the `unpkg.com` CDN.
- Bundle size: Note that the old `...WorkerLoader` classes are still available. Using these can save even more bundle space since during tree-shaking since they do not depend on the non-worker parser.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const tileMap = {}; // Map the contentUri -> tile so we can unload/set visibilit
let viewer;

// set up loaders
setLoaderOptions({worker: false});
setLoaderOptions({core: {worker: false}});

export async function loadTileset({tilesetUrl, ionAssetId, ionAccessToken, viewerInstance}) {
let fetchOptions = null;
Expand Down
6 changes: 4 additions & 2 deletions examples/experimental/slpk-in-browser/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,10 @@ export default function App() {
}
const loadOptions = {
i3s: {coordinateSystem: COORDINATE_SYSTEM.LNGLAT_OFFSETS},
fetch: fileSystem.fetch.bind(fileSystem),
worker: false
core: {
fetch: fileSystem.fetch.bind(fileSystem),
worker: false
}
};
// @ts-expect-error
const layers = new CustomTile3DLayer({
Expand Down
8 changes: 5 additions & 3 deletions examples/experimental/slpk-via-range-requests/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export default function App() {
}

const createFileSystem = async (url: string) => {
console.log("New url", url)
console.log('New url', url);
const fileProvider = await FileProvider.create(new HttpFile(url));
const archive = await parseSLPKArchive(fileProvider, undefined, url);
const fileSystem = new ZipFileSystem(archive);
Expand Down Expand Up @@ -78,8 +78,10 @@ export default function App() {
}
const loadOptions = {
i3s: {coordinateSystem: COORDINATE_SYSTEM.LNGLAT_OFFSETS},
fetch: fileSystem.fetch.bind(fileSystem),
worker: false
core: {
fetch: fileSystem.fetch.bind(fileSystem),
worker: false
}
};
// @ts-expect-error
const layers = new CustomTile3DLayer({
Expand Down
10 changes: 6 additions & 4 deletions examples/website/geospatial/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type {Example} from './examples';
import {INITIAL_LOADER_NAME, INITIAL_EXAMPLE_NAME, EXAMPLES} from './examples';

import {Table, GeoJSON} from '@loaders.gl/schema';
import {Loader, load /* registerLoaders */} from '@loaders.gl/core';
import {Loader, load, LoaderOptions} from '@loaders.gl/core';
import {GeoArrowLoader} from '@loaders.gl/arrow';
import {GeoParquetLoader, installBufferPolyfill, preloadCompressions} from '@loaders.gl/parquet';
import {FlatGeobufLoader} from '@loaders.gl/flatgeobuf';
Expand Down Expand Up @@ -47,8 +47,10 @@ const LOADERS = [
] as const;

const LOADER_OPTIONS = {
worker: false,
limit: 1800000,
core: {
worker: false,
limit: 1800000
},
modules: {
'zstd-codec': ZstdCodec
},
Expand Down Expand Up @@ -79,7 +81,7 @@ const LOADER_OPTIONS = {
tcx: {
shape: 'geojson-table'
}
} as const;
} as const satisfies LoaderOptions;

const VIEW_STATE = {
height: 600,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ export async function extractGLTF(
}
if (tile.gltfUrl) {
const {fetch} = context;
const response = await fetch(tile.gltfUrl, options);
// @ts-expect-error TODO - This may not work
const response = await fetch(tile.gltfUrl, options?.core);
tile.gltfArrayBuffer = await response.arrayBuffer();
tile.gltfByteOffset = 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ test('loadDraco# Pass options to draco loader properly', async (t) => {
// TODO - we need a test file with 64 bit data
test.skip('point cloud tile#64bit attribute', async (t) => {
const POINTCLOUD_64BIT_URL = '@loaders.gl/3d-tiles/test/data/64-bit-attribute/1.pnts';
const result = await load(POINTCLOUD_64BIT_URL, Tiles3DLoader, {worker: false});
const result = await load(POINTCLOUD_64BIT_URL, Tiles3DLoader, {core: {worker: false}});
t.ok(result.attributes.gpstime instanceof Float64Array);
t.equal(result.attributes.gpstime.length, 31648);
t.end();
Expand Down
10 changes: 5 additions & 5 deletions modules/arrow/test/arrow-loader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ test('ArrowLoader#loader conformance', (t) => {
});

test('ArrowLoader#parseSync(simple.arrow)', async (t) => {
const arrowTable = await parse(fetchFile(ARROW_SIMPLE), ArrowLoader, {worker: false});
const arrowTable = await parse(fetchFile(ARROW_SIMPLE), ArrowLoader, {
core: {worker: false}
});
// Check loader specific results
t.equal(arrowTable.shape, 'columnar-table');
if (arrowTable.shape === 'columnar-table') {
Expand All @@ -50,10 +52,8 @@ test('ArrowLoader#parseSync(simple.arrow)', async (t) => {

test('ArrowLoader#parseSync(simple.arrow) type="object-row-table"', async (t) => {
const rowFormatTable = await parse(fetchFile(ARROW_SIMPLE), ArrowLoader, {
worker: false,
arrow: {
shape: 'object-row-table'
}
core: {worker: false},
arrow: {shape: 'object-row-table'}
});
t.equal(rowFormatTable.shape, 'object-row-table');
if (rowFormatTable.shape === 'object-row-table') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,8 @@ async function testGetBinaryGeometriesFromArrow(
expectedBinaryGeometries
): Promise<void> {
const arrowTable = await load(arrowFile, ArrowLoader, {
worker: false,
arrow: {
shape: 'arrow-table'
}
core: {worker: false},
arrow: {shape: 'arrow-table'}
});

t.equal(arrowTable.shape, 'arrow-table');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ async function testParseFromArrow(
expectedGeojson: FeatureCollection
): Promise<void> {
const arrowTable = await load(arrowFile, ArrowLoader, {
worker: false,
arrow: {
shape: 'arrow-table'
}
core: {worker: false},
arrow: {shape: 'arrow-table'}
});

t.equal(arrowTable.shape, 'arrow-table');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ async function testConversion(
expectedGeojson: FeatureCollection
): Promise<void> {
const table = await parse(fetchFile(arrowFile), GeoArrowLoader, {
worker: false,
arrow: {
shape: 'geojson-table'
}
core: {worker: false},
arrow: {shape: 'geojson-table'}
});

t.equal(table.shape, 'geojson-table');
Expand Down
9 changes: 6 additions & 3 deletions modules/core/src/lib/api/parse-in-batches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export async function parseInBatches(
// Signature: parseInBatches(data, options, url) - Uses registered loaders
if (!Array.isArray(loaders) && !isLoaderObject(loaders)) {
context = undefined; // context not supported in short signature
options = loaders as LoaderOptions;
options = loaders as unknown as LoaderOptions;
loaders = undefined;
}

Expand Down Expand Up @@ -161,7 +161,10 @@ async function parseToOutputIterator(
const inputIterator = await getAsyncIterableFromData(data, options);

// Apply any iterator transforms (options.transforms)
const transformedIterator = await applyInputTransforms(inputIterator, options?.transforms || []);
const transformedIterator = await applyInputTransforms(
inputIterator,
options?.core?.transforms || []
);

// If loader supports parseInBatches, we are done
if (loader.parseInBatches) {
Expand All @@ -184,7 +187,7 @@ async function* parseChunkInBatches(
arrayBuffer,
loader,
// TODO - Hack: supply loaders MIME type to ensure we match it
{...options, mimeType: loader.mimeTypes[0]},
{...options, core: {...options?.core, mimeType: loader.mimeTypes[0]}},
context
);

Expand Down
15 changes: 10 additions & 5 deletions modules/core/src/lib/api/select-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ export async function selectLoader(
}

// First make a sync attempt, disabling exceptions
let loader = selectLoaderSync(data, loaders, {...options, nothrow: true}, context);
let loader = selectLoaderSync(
data,
loaders,
{...options, core: {...options?.core, nothrow: true}},
context
);
if (loader) {
return loader;
}
Expand Down Expand Up @@ -126,8 +131,8 @@ function selectLoaderInternal(
let reason: string = '';

// if options.mimeType is supplied, it takes precedence
if (options?.mimeType) {
loader = findLoaderByMIMEType(loaders, options?.mimeType);
if (options?.core?.mimeType) {
loader = findLoaderByMIMEType(loaders, options?.core?.mimeType);
reason = `match forced by supplied MIME type ${options?.mimeType}`;
}

Expand All @@ -146,8 +151,8 @@ function selectLoaderInternal(
reason = reason || (loader ? `matched initial data ${getFirstCharacters(data)}` : '');

// Look up loader by fallback mime type
if (options?.fallbackMimeType) {
loader = loader || findLoaderByMIMEType(loaders, options?.fallbackMimeType);
if (options?.core?.fallbackMimeType) {
loader = loader || findLoaderByMIMEType(loaders, options?.core?.fallbackMimeType);
reason = reason || (loader ? `matched fallback MIME type ${type}` : '');
}

Expand Down
69 changes: 47 additions & 22 deletions modules/core/src/lib/loader-utils/option-defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,59 @@ import {isBrowser} from '@loaders.gl/loader-utils';
import {ConsoleLog} from './loggers';

export const DEFAULT_LOADER_OPTIONS: LoaderOptions = {
// baseUri
fetch: null,
mimeType: undefined,
nothrow: false,
log: new ConsoleLog(), // A probe.gl compatible (`log.log()()` syntax) that just logs to console
useLocalLibraries: false,

CDN: 'https://unpkg.com/@loaders.gl',
worker: true, // By default, use worker if provided by loader.
maxConcurrency: 3, // How many worker instances should be created for each loader.
maxMobileConcurrency: 1, // How many worker instances should be created for each loader on mobile devices.
reuseWorkers: isBrowser, // By default reuse workers in browser (Node.js refuses to terminate if browsers are running)
_nodeWorkers: false, // By default do not support node workers
_workerType: '', // 'test' to use locally generated workers

limit: 0,
_limitMB: 0,
batchSize: 'auto',
batchDebounceMs: 0,
metadata: false, // TODO - currently only implemented for parseInBatches, adds initial metadata batch,
transforms: []
core: {
// baseUri
fetch: null,
mimeType: undefined,
nothrow: false,
log: new ConsoleLog(), // A probe.gl compatible (`log.log()()` syntax) that just logs to console
useLocalLibraries: false,

CDN: 'https://unpkg.com/@loaders.gl',
worker: true, // By default, use worker if provided by loader.
maxConcurrency: 3, // How many worker instances should be created for each loader.
maxMobileConcurrency: 1, // How many worker instances should be created for each loader on mobile devices.
reuseWorkers: isBrowser, // By default reuse workers in browser (Node.js refuses to terminate if browsers are running)
_nodeWorkers: false, // By default do not support node workers
_workerType: '', // 'test' to use locally generated workers

limit: 0,
_limitMB: 0,
batchSize: 'auto',
batchDebounceMs: 0,
metadata: false, // TODO - currently only implemented for parseInBatches, adds initial metadata batch,
transforms: []
}
};

export const REMOVED_LOADER_OPTIONS = {
// baseUri
fetch: 'core.fetch',
mimeType: 'core.mimeType',
nothrow: 'core.nothrow',
log: 'core.log',
useLocalLibraries: 'core.useLocalLibraries',

CDN: 'core.CDN',
worker: 'core,worker',
maxConcurrency: 'core.maxConcurrency',
maxMobileConcurrency: 'core.maxMobileConcurrency',
reuseWorkers: 'core.reuseWorkers',
_nodeWorkers: 'core.nodeWorkers',
_workerType: 'core._workerType',

limit: 'core.limit',
_limitMB: 'core._limitMB',
batchSize: 'core.batchSize',
batchDebounceMs: 'core.batchDebounceMs',
metadata: 'core.metadata',
transforms: 'core.transforms',

// Older deprecations
throws: 'nothrow',
dataType: '(no longer used)',
uri: 'baseUri',
// Warn if fetch options are used on top-level
// Warn if fetch options are used on toplevel
method: 'fetch.method',
headers: 'fetch.headers',
body: 'fetch.body',
Expand Down
13 changes: 6 additions & 7 deletions modules/core/src/lib/loader-utils/option-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,7 @@ function validateOptions(options: LoaderOptions, loaders: Loader[]): void {
validateOptionsObject(options, null, DEFAULT_LOADER_OPTIONS, REMOVED_LOADER_OPTIONS, loaders);
for (const loader of loaders) {
// Get the scoped, loader specific options from the user supplied options
const idOptions: Record<string, unknown> = ((options && options[loader.id]) || {}) as Record<
string,
unknown
>;
const idOptions: Record<string, unknown> = (options && options[loader.id]) || {};

// Get scoped, loader specific default and deprecated options from the selected loader
const loaderOptions = (loader.options && loader.options[loader.id]) || {};
Expand Down Expand Up @@ -169,8 +166,8 @@ function normalizeOptionsInternal(
addUrlOptions(mergedOptions, url);

// LOGGING: options.log can be set to `null` to defeat logging
if (mergedOptions.log === null) {
mergedOptions.log = new NullLog();
if (mergedOptions.core?.log === null) {
mergedOptions.core = {...mergedOptions.core, log: new NullLog()};
}

mergeNestedFields(mergedOptions, getGlobalLoaderOptions());
Expand Down Expand Up @@ -209,6 +206,8 @@ function mergeNestedFields(mergedOptions: LoaderOptions, options: LoaderOptions)
*/
function addUrlOptions(options: LoaderOptions, url?: string): void {
if (url && !('baseUri' in options)) {
options.baseUri = url;
options.core ||= {};
// @ts-expect-error TODO - remove
options.core.baseUri = url;
}
}
2 changes: 1 addition & 1 deletion modules/core/test/lib/loader-utils/option-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const TEST_CASES = [
},
{
loader: LASLoader,
options: {las: {skip: 10}, worker: false},
options: {las: {skip: 10}, core: {worker: false}},
assert: (t, options) => {
t.equal(options.las.skip, 10);
t.equal(options.worker, false);
Expand Down
Loading
Loading