#7670: Simplify creation of isolated widgets (IsolatedComponent)
#8151
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,16 +55,25 @@ const config = { | |
| silent: true, | ||
| testEnvironment: "./src/testUtils/FixJsdomEnvironment.js", | ||
| modulePaths: ["/src"], | ||
| moduleFileExtensions: [ | ||
| "ts", | ||
| "tsx", | ||
| "js", | ||
| "jsx", | ||
| "mjs", | ||
| "yaml", | ||
| "yml", | ||
| "json", | ||
| ], | ||
| modulePathIgnorePatterns: ["<rootDir>/headers.json", "<rootDir>/dist/"], | ||
| testPathIgnorePatterns: ["/end-to-end-tests"], | ||
| transform: { | ||
| "\\.[jt]sx?$": "@swc/jest", | ||
| "\\.mjs$": "@swc/jest", | ||
| "\\.ya?ml$": "yaml-jest-transform", | ||
| "\\.txt$": "<rootDir>/src/testUtils/rawJestTransformer.mjs", | ||
| // Note: `?param` URLs aren't supported here: https://github.com/jestjs/jest/pull/6282 | ||
| // You can only use a mock via `moduleNameMapper` for these. | ||
| }, | ||
| transformIgnorePatterns: [`node_modules/(?!${esmPackages.join("|")})`], | ||
| setupFiles: [ | ||
|
|
@@ -89,11 +98,15 @@ const config = { | |
| "!**/vendor/**", | ||
| ], | ||
| moduleNameMapper: { | ||
| "^@contrib/([^?]+)": "<rootDir>/contrib/$1", | ||
| "^@schemas/([^?]+)": "<rootDir>/schemas/$1", | ||
|
|
||
| "\\.s?css$": "identity-obj-proxy", | ||
| "\\.(gif|svg|png)$": "<rootDir>/src/__mocks__/stringMock.js", | ||
|
|
||
| "\\?loadAsUrl$": "<rootDir>/src/__mocks__/stringMock.js", | ||
| "\\?loadAsText$": "<rootDir>/src/__mocks__/stringMock.js", | ||
|
There was a problem hiding this comment. Does this conflict with the There was a problem hiding this comment. Given that tests and snapshots don't need to be updated, I'm guessing that it's not actually being applied nor needed. Anyway I think these 4 above should be moved below. There was a problem hiding this comment. I just wondered if it's related to the changes to imports in There was a problem hiding this comment. Sort of. I dropped it from that test file because:
Since I cleaned up line 109 as well: 1334a91 |
||
| "\\?loadAsComponent$": "<rootDir>/src/__mocks__/stringMock.js", | ||
|
|
||
| // Auto-mocks. See documentation in ./src/__mocks__/readme.md | ||
| "^@/(.*)$": ["<rootDir>/src/__mocks__/@/$1", "<rootDir>/src/$1"], | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,8 @@ import { sortBy, isPlainObject } from "lodash"; | |
| import { type BrickArgs, type BrickOptions } from "@/types/runtimeTypes"; | ||
| import { canParseUrl } from "@/utils/urlUtils"; | ||
| import { propertiesToSchema } from "@/utils/schemaUtils"; | ||
| import IsolatedComponent from "@/components/IsolatedComponent"; | ||
| import type TreeNode from "primereact/treenode"; | ||
|
|
||
| interface Item { | ||
| key: string; | ||
|
|
@@ -115,13 +117,21 @@ export class PropertyTableRenderer extends RendererABC { | |
| ); | ||
|
|
||
| async render({ data }: BrickArgs, { ctxt }: BrickOptions) { | ||
| const PropertyTree: React.FC<{ value: TreeNode[] }> = ({ value }) => ( | ||
| <IsolatedComponent | ||
| name="PropertyTree" | ||
| lazy={async () => | ||
| import( | ||
| /* webpackChunkName: "isolated/PropertyTree" */ | ||
| "./PropertyTree" | ||
| ) | ||
| } | ||
| factory={(PropertyTree) => <PropertyTree value={value} />} | ||
| /> | ||
|
Comment on lines
+121
to
+130
There was a problem hiding this comment. This is the format I ended up with, I think it's compact and readable enough, which is important due to all the restrictions:
There was a problem hiding this comment. I'd like to document some of these requirements all in one place somewhere (e.g. it's not clear just looking at IsolatedComponent.tsx that you'd need to add that component to isolatedComponentList). Either in the docstring directly or in a README that's referenced in the docstring. If I'm looking to add a new isolated component, what are the steps that need to be taken and out-of-the box behaviors or assumptions that are made? There was a problem hiding this comment. Yes definitely it needs some documentation and a lint rule, but:
I can't think of anything that breaks that isn't already caught but the build.
Since "PropertyTree" is now loaded as an isolated component, it needs to There was a problem hiding this comment.
I just checked, each prop is already documented and it includes the requirement: pixiebrix-extension/src/components/IsolatedComponent.tsx Lines 61 to 70 in 1334a91 Also this: pixiebrix-extension/src/components/IsolatedComponent.tsx Lines 108 to 112 in 1334a91 |
||
| ); | ||
|
|
||
| return { | ||
| Component: PropertyTree, | ||
| props: { | ||
| value: shapeData(data ?? ctxt), | ||
| }, | ||
|
|
||
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 updated these to avoid confusion. See comment/link.