-
Notifications
You must be signed in to change notification settings - Fork 195
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
chore(examples): Example with loading of slpk in browser implementation #2904
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. How big SPLKs have you tested it on?
"@loaders.gl/loader-utils": "^4.0.0", | ||
"@loaders.gl/mvt": "^4.0.0", | ||
"@loaders.gl/terrain": "^4.0.0", | ||
"@testing-library/jest-dom": "^5.17.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a LOT of dependencies. Do we really need the test libraries for an example? Can they go into devDependencies
?
"react-app/jest" | ||
] | ||
}, | ||
"browserslist": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need these?
<title>React App</title> | ||
</head> | ||
<body> | ||
<noscript>You need to enable JavaScript to run this app.</noscript> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like some really old boilerplate... What browser doesn't have JS enabled these days?
* Gets an unsigned 8-bit integer at the specified byte offset from the start of the file. | ||
* @param offset The offset, in bytes, from the start of the file where to read the data. | ||
*/ | ||
async getUint8(offset: number | bigint): Promise<number> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel you should separate this out of your File implementation.
You read ArrayBuffer
's from the FIle, and you create DataView
s on those ArrayBuffers to read individual binary values.
If you want a help class to wrap DataView that is fine, but I would suggest keeping it separate from File
to keep things separate and simple
"esModuleInterop": true, | ||
"allowSyntheticDefaultImports": true, | ||
"strict": true, | ||
"forceConsistentCasingInFileNames": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really need all these options? Simpler is better.
I will fix comments in the next PR |
No description provided.