-
Notifications
You must be signed in to change notification settings - Fork 23
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
Generate mock data lazily and improve API #1537
Conversation
d56fba6
to
7d128b3
Compare
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.
First round of comments, but I still need to make some changes before the review.
76b3e9a
to
318f52b
Compare
/approve |
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.
Seems all fine to me ! Well done !
apps/storybook/src/Utilities.mdx
Outdated
The library also exposes the `makeMockFile` utility function that is used by the [mock demo](https://h5web.panosc.eu/mock) to generate mock metadata and values. | ||
You can retrieve entities from the generated mock file by their paths using the `findMockEntity` utility function: | ||
|
||
```ts | ||
import { findMockEntity, getMockDataArray } from '@h5web/lib'; | ||
import { findMockEntity, makeMockFile } from '@h5web/lib'; | ||
|
||
const entity = findMockEntity('/nD_datasets/twoD'); | ||
const dataArray = getMockDataArray('/nD_datasets/twoD'); | ||
const mockFile = makeMockFile(); | ||
const entity = findMockEntity(mockFile, '/nD_datasets/twoD'); |
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.
Yeah, not sure it is actually relevant to document this.
I realised that the
mockValues
andmockMetadata
objects were not properly tree-shaken in consumer applications of@h5web/lib
or@h5web/app
(at least with ones using Vite as their bundler). As a result, those two giant data structures were being created on every page load of myHDF5, just to name one.Tree-shaking works better (only?) with functions, so the idea is to turn
mockValues
andmockMetadata
into functions. This has benefits even in projects that do need these two structures (e.g. in the demo project), since it means that we can generate them lazily, when they are actually needed (e.g. when navigating to the mock demo).I had tried to generate the mock metadata and values lazily before, but never really got to a point that I found satisfactory. The API to access or generate entities and arrays that we use in our Jest test suites and in the Storybook was always quite convoluted.
This time, I had the idea of generating ndarrays instead of plain, nested JS arrays in
mockValues
, which unlocked a lot of things, thanks to the simple fact that ndarrays store the shape of their underlying arrays. For instance, this means that we:[20, 41]
...) when we generate the mock metadata;Additionally, since the internal array inside an ndarray is already flattened, we no longer need to deal with this in
MockApi
.The PR is huge particularly because I took the opportunity to clean the metadata a bit and to shorten the metadata utility functions to make the whole thing a bit more readable.