Skip to content

Conversation

heath-freenome
Copy link
Member

@heath-freenome heath-freenome commented Apr 30, 2025

Reasons for making this change

Updated as many of the 3rd Party libraries as possible

  • Updated the root package.json to consolidate the dev dependencies from all of the sub-packages like:
    • Typescript, rollup, jest, react, types and babel
    • Also bumped all of the libraries that were out-of-date
  • Updated the individual package.json files to update libraries and to remove duplicated dev dependencies that are in the root package.json
  • Updated types in @rjsf/utils and @rjsf/core due to typescript and lodash type updates
  • Updated DemoFrame in playground to work with the latest version of react-frame-component
  • Updated all the snapshots for themes that needed them due to library updates
  • Updated docs libraries, including fixing up .md files that caused problems with the latest version of Docusaurus
  • Made @rjsf/utils isObject() a type guard

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npx nx run-many --target=build --exclude=@rjsf/docs && npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@@ -74,7 +64,7 @@ export default function DemoFrame(props: DemoFrameProps) {
);
setContainer(instanceRef.current.contentDocument.body);
setWindow(() => instanceRef.current.contentWindow);
}, []);
}, [instanceRef]);
Copy link
Contributor

@nickgros nickgros May 1, 2025

Choose a reason for hiding this comment

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

I don't think this will do anything because the value of instanceRef will never change

Copy link
Member Author

Choose a reason for hiding this comment

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

technically the instanceRef.current can be changed during the mount/unmount... it is technically correct to have it there

Copy link
Contributor

Choose a reason for hiding this comment

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

But even when current changes, instanceRef will be the same object

Copy link
Member Author

Choose a reason for hiding this comment

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

True... And having it as a dependency doesn't hurt and is technically correct

Comment on lines -56 to -64
const handleRef = useCallback(
(ref: any) => {
instanceRef.current = {
contentDocument: ref ? ref.node.contentDocument : null,
contentWindow: ref ? ref.node.contentWindow : null,
};
},
[instanceRef],
);
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it the instanceRef is now directly set and does the same thing

// Check if the parent schema has a const property defined AND we are supporting const as defaults, then we
// should always return the computedDefault since it's coming from the const.
const hasParentConst = isObject(parentConst) && (parentConst as JSONSchema7Object)[key] !== undefined;
const hasConst =
((isObject(propertySchema) && CONST_KEY in propertySchema) || hasParentConst) &&
((typeof propertySchema === 'object' && CONST_KEY in propertySchema) || hasParentConst) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

why was this changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

type checking. Our isObject() function did not work as a type guard

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not update isObject to be a typeguard? Think it would be as simple as

export default function isObject(thing: any): thing is object {

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, let me try that

Copy link
Member Author

Choose a reason for hiding this comment

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

That works!

Comment on lines -1016 to -1019
/** Allows RJSF to override the default field implementation by specifying either the name of a field that is used
* to look up an implementation from the `fields` list or an actual one-off `Field` component implementation itself
*/
'ui:field'?: Field<T, S, F> | string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, MakeUIType will handle this with field defined in UIOptionsBaseType

@@ -76,38 +76,17 @@
"rc-picker": "2.7.6"
},
"devDependencies": {
"@ant-design/icons": "^5.6.1",
"@babel/cli": "^7.23.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

fyi, if we switch to pnpm then we may still need to explicitly list the dependencies we are using in each package

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's wait until we do that then. I wanted to reduce the size of our package files right now

Copy link
Contributor

@nickgros nickgros left a comment

Choose a reason for hiding this comment

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

had some questions, not blocking approval

Updated as many of the 3rd Party libraries as possible
- Updated the root `package.json` to consolidate the dev dependencies from all of the sub-packages like:
  - Typescript, rollup, jest, react, types and babel
  - Also bumped all of the libraries that were out-of-date
- Updated the individual `package.json` files to update libraries and to remove duplicated dev dependencies that are in the root `package.json`
- Updated types in `@rjsf/utils` and `@rjsf/core` due to typescript and lodash type updates
- Updated `DemoFrame` in `playground` to work with the latest version of `react-frame-component`
- Updated all the snapshots for themes that needed them due to library updates
… problems with the latest version of Docusaurus
@heath-freenome heath-freenome force-pushed the Update-3rd-party-libs branch from 136b121 to 1923b75 Compare May 1, 2025 18:07
@heath-freenome heath-freenome merged commit 1ecd15b into rjsf-v6 May 1, 2025
4 checks passed
@heath-freenome heath-freenome deleted the Update-3rd-party-libs branch May 1, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants