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

Widgets unusable under Antd v5 using a dark theme (wrong ConfigProvider context passed down) #4129

Closed
4 tasks done
momesana opened this issue Mar 15, 2024 · 3 comments · Fixed by #4132
Closed
4 tasks done

Comments

@momesana
Copy link
Contributor

momesana commented Mar 15, 2024

Prerequisites

What theme are you using?

antd

Version

5.x

Current Behavior

When using Antd v5 in combination with a dark theme, the widgets provided by rjsfs/antd are unusuable:
image

Expected Behavior

Colors, background colors etc. should match the selected antd theme.

Steps To Reproduce

Just use the RJFS/antd widgets in combination with a dark theme. For convenience you'll find different ways to easily reproduce the error.

Using stackblitz:

https://stackblitz.com/~/github.com/momesana/rjsf-darktheme-bug. You may have to use pnpm run dev from StackBlitz's terminal environment to launch it.

Using a github repository (using a webpack based custom starter using pnpm)

Here is a repository to highlight the issue: https://github.com/momesana/rjsf-darktheme-bug. Clone the repo and then issue pnpm install followed by pnpm run dev which should open the developoment environment on localhost:8080.

Using vite and npm

Alternatively, if you don't like the opinionated custom webpack-based starter above, you can achieve the same with vite by:

  1. running npm create vite@latest rjsf-darktheme-bug -- --template react-ts
  2. change into rjsf-darktheme-bug and install deps: cd rjsf-darktheme-bug && npm install
  3. now install rjsf, antd and other required packages: npm install --save @rjsf/core @rjsf/utils @rjsf/validator-ajv8 @rjsf/antd antd dayjs @ant-design/icons
  4. changing the content of src/App.tsx to match that of https://github.com/momesana/rjsf-darktheme-bug/blob/main/src/App.tsx either by manually replacing the content or running curl https://raw.githubusercontent.com/momesana/rjsf-darktheme-bug/main/src/App.tsx -o src/App.tsx from within the project root folder
  5. lastly run npm run dev

In short (with npm and curl being available):

cd /tmp && \
npm create vite@latest rjsf-darktheme-bug -- --template react-ts && \
cd rjsf-darktheme-bug && \
npm install && \
npm install --save @rjsf/core @rjsf/utils @rjsf/validator-ajv8 @rjsf/antd antd dayjs @ant-design/icons && \
curl https://raw.githubusercontent.com/momesana/rjsf-darktheme-bug/main/src/App.tsx -o src/App.tsx && \
npm run dev

Environment

- OS: linux
- Node: v20LTS
- npm: actually pnpm v8

Anything else?

When I clone the rjsf repository and change the way the antd form components are imported from deep imports to named imports as follows, then build rjsfs and link it with npm link <path-to-the-self-built-rjfs-antd-package> the issue goes away:

...
-import Button from 'antd/lib/button';
-import Col from 'antd/lib/col';
-import Row from 'antd/lib/row';
+import { Button, Col, Row } from 'antd';
...

The result being this (with the primaryColor set to crimson red):
image

I don't have much experience with building libraries and the requirements it entails and I assume the imports where the way they were for a particular reason (facilitating tree-shaking etc.) though according to this post on the antd github repo using the named exports i. e. importing directly from 'antd' is the preferred way to handle this. This answer here also seems to corroborate it and is from one of the active antd developers.

In the case that the deep imports weren't that intentional (and also for anyone interested in testing out the proposed fix as a means to get to the bottom of things) here is a PR with the above mentioned changes.

Something peculiar I noticed that I can't yet wrap my head around is that using stackblitz without webpack/vite etc. seems to work fine:
https://stackblitz.com/edit/react-cnrcej?file=demo.tsx,index.html
I assume this is due to Stackblitz directly importing the es modules instead of bundling things as webpack and vite do. It's just conjecture though.

Edit:
I've just created a very simple vite project with just antd in it using a dark theme and a single Antd Button. As soon as I switch to a default deep import via import Button from 'antd/lib/button'; as opposed to import { Button } from 'antd'; the theming stops working, so either it's an antd bug or these deep imports aren't meant to be used.

2nd Edit:
After reading this and this, I assume the named exports are supposed to be used instead of the deep default imports. To clarify this, I've also started this discussion thread on the antd github project.

3rd Edit:
Ok, the reason why this doesn't work is that the used components must have a useContext(C) or a ContextConsumer that must be identical to the one used to create the context. The only way to make the inputs receive the correct theming data from the context is to also import the Context via a deep import i.e. import ConfigProvider from "antd/lib/config-provider";.
This evidently isn't practical as client code will most likely have used named imports for importing the antd components and thus the code really needs to be fixed in react-json-schema-forms.

I've updated the above PR and removed all deep imports and rewritten the part that used the ConfigConsumer to use the ConfigProvider.ConfigContext so that there is no more dependence on any unexposed antd internals. Needless to say, the theming now would break for client code that uses deep imports but that shouldn't exist and thus rather constitutes a bug in the client code that we don't need to account for here.

@momesana momesana added bug needs triage Initial label given, to be assigned correct labels and assigned labels Mar 15, 2024
@heath-freenome heath-freenome added help wanted and removed needs triage Initial label given, to be assigned correct labels and assigned labels Mar 15, 2024
@momesana momesana changed the title Widgets unusable under Antd v5 using a dark theme Widgets unusable under Antd v5 using a dark theme (wrong ConfigProvider context passed down) Mar 17, 2024
@momesana
Copy link
Contributor Author

@heath-freenome, any chance we might get the PR I've created approved/merged?

@momesana
Copy link
Contributor Author

momesana commented Mar 19, 2024

If past experience with PRs for rjsf is an indication, I'd say the prospects of the fix being reviewed/approved/employed soon or the Issue being addressed separately in a timely manner are rather slim. So, anyone having this issue can as a (hopefully temporary) workaround, wrap the subtree containing the RJFS forms in a second config provider that is imported via a deep import: import TemporaryFixConfigProvider from 'antd/lib/config-provider'; i.e.:

import { ConfigProvider } from 'antd';
import TemporaryFixConfigProvider from 'antd/lib/config-provider';
...
return (
    <ConfigProvider ...>
        <TemporaryFixConfigProvider ...>
            ...
        </TemporaryFixConfigProvider>
    </ConfigProvider>
);

@heath-freenome
Copy link
Member

@momesana One small change and I'll approve/merge/release this change by Friday

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants