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

refactor: upgrade Jotai to v2 and use yarn v4 #55

Open
wants to merge 10 commits into
base: alpha
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
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ jobs:
key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
restore-keys: |
${{ runner.os }}-yarn-
- name: corepack
run: corepack enable
- name: Run install
run: yarn install --frozen-lockfile
- name: Run lint
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ dist
dist-ssr
*.local

.yarn

# Editor directories and files
.vscode/*
!.vscode/extensions.json
Expand Down
1 change: 1 addition & 0 deletions .yarnrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
nodeLinker: node-modules
13 changes: 6 additions & 7 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@
"cesium-dnd": "1.1.0",
"csv-parse": "5.5.2",
"d3": "7.9.0",
"escape-string-regexp": "4.0.0",
"fast-xml-parser": "4.3.2",
"framer-motion": "11.0.27",
"geojson": "0.5.0",
"jotai": "1.12.1",
"jotai": "2.9.3",
"js-md5": "0.7.3",
"jsep": "1.3.8",
"jsonpath-plus": "7.2.0",
Expand All @@ -71,7 +72,7 @@
"react-dnd-html5-backend": "16.0.1",
"react-error-boundary": "4.0.11",
"react-nl2br": "1.0.4",
"react-use": "17.5.0",
"react-use": "17.5.1",
"resium": "1.18.2",
"suspend-react": "0.1.3",
"tailwind-merge": "2.5.2",
Expand All @@ -80,11 +81,10 @@
"tiny-invariant": "1.3.3",
"use-callback-ref": "1.3.2",
"use-custom-compare": "1.4.0",
"uuid": "9.0.1",
"uuid": "10.0.0",
"xstate": "4.38.2"
},
"devDependencies": {
"@apollo/client": "3.8.1",
"@chromatic-com/storybook": "^1.3.3",
"@emotion/jest": "11.11.0",
"@storybook/addon-essentials": "8.0.8",
Expand Down Expand Up @@ -117,6 +117,7 @@
"clsx": "2.1.1",
"eslint": "8.57.0",
"eslint-config-reearth": "0.3.0",
"eslint-plugin-import": "2.31.0",
"eslint-plugin-react-hooks": "4.6.0",
"eslint-plugin-react-refresh": "0.4.6",
"eslint-plugin-storybook": "0.8.0",
Expand All @@ -135,7 +136,5 @@
"vitest": "1.0.4",
"web-streams-polyfill": "3.2.1"
},
"resolutions": {
"jackspeak": "2.1.1"
}
"packageManager": "[email protected]"
}
23 changes: 12 additions & 11 deletions src/mantle/atoms/compute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,17 @@ test("computeAtom", async () => {
status: "ready",
features: [],
originalFeatures: [],
}),
await waitFor(() =>
expect(result.current.result).toEqual({
id: "xxx",
layer,
status: "ready",
features: [],
originalFeatures: [],
}),
);
});

await waitFor(() =>
expect(result.current.result).toEqual({
id: "xxx",
layer,
status: "ready",
features: [],
originalFeatures: [],
}),
);

// delete delegatedDataTypes
act(() => {
Expand Down Expand Up @@ -187,7 +188,7 @@ test("computeAtom", async () => {
layer,
status: "fetching",
features: [...toComputedFeature(features), ...toComputedFeature(features2)],
originalFeatures: [...features, ...features3],
originalFeatures: [...features, ...features2],
Copy link
Contributor Author

@airslice airslice Oct 9, 2024

Choose a reason for hiding this comment

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

This may not follows the origin idea how it should work.
I think it's coming from the upgrade Jotai to v2 around async atom maybe.
However it seems also make sense to have this result if set data is async, but if it's by design that the orignalFeatures should be updated immediatly, we need to investigate the cause and fix it.
Also i don't understand how feature2 been removed from oringalFeatures in this test.

});

await waitFor(() =>
Expand Down
4 changes: 2 additions & 2 deletions src/mantle/atoms/compute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function computeAtom(cache?: typeof globalDataFeaturesCache) {
});

const layer = atom<Layer | undefined>(undefined);
const overrides = atom<Record<string, any> | undefined, Record<string, any> | undefined>(
const overrides = atom<Record<string, any> | undefined, [Record<string, any> | undefined], void>(
undefined,
(_, set, value) => {
set(overrides, pick(value, appearanceKeys));
Expand Down Expand Up @@ -308,7 +308,7 @@ export function computeAtom(cache?: typeof globalDataFeaturesCache) {
await set(compute, { forceUpdateData: true });
});

return atom<ComputedLayer | undefined, Command>(
return atom<ComputedLayer | undefined, [Command], void>(
g => g(get),
async (_, s, value) => {
switch (value.type) {
Expand Down
10 changes: 1 addition & 9 deletions src/test/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
// TODO: remove MockedProvider after double-checking the test here is not necessary
// import { MockedProvider as MockedGqlProvider, MockedResponse } from "@apollo/client/testing";
import { render as rtlRender } from "@testing-library/react";
import { vitest } from "vitest";

// import { Provider as I18nProvider } from "../services/i18n";

// react-inlinesvg is not displayed in test.
// see detail: https://github.com/gilbarbara/react-inlinesvg/issues/145
vitest.mock("react-inlinesvg", () => {
Expand All @@ -22,11 +18,7 @@ vitest.mock("react-inlinesvg", () => {
};
});

const render = (
ui: React.ReactElement,
// queryMocks?: readonly MockedResponse<Record<string, any>>[],
{ ...renderOptions } = {},
) => {
const render = (ui: React.ReactElement, { ...renderOptions } = {}) => {
const Wrapper: React.FC<{ children?: React.ReactNode }> = ({ children }) => {
return <>{children}</>;
};
Expand Down
Loading
Loading