refactor: replace legacy ErrorMessage.js with messages/ErrorMessage.tsx#7026
refactor: replace legacy ErrorMessage.js with messages/ErrorMessage.tsx#7026SahilJat wants to merge 1 commit intoFlagsmith:mainfrom
Conversation
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.
Once credits are available, reopen this pull request to trigger a review.
|
@SahilJat is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
talissoncosta
left a comment
There was a problem hiding this comment.
Hey @SahilJat, thanks for tackling this — nice cleanup removing the old class component and updating the imports! 🎉
I spotted one issue: 4 files still import from the old path (./ErrorMessage), which breaks the build. These were missed because they live in web/components/ alongside the deleted file:
web/components/NewVersionWarning.tsxweb/components/SamlForm.jsweb/components/UsersGroups.tsxweb/components/EditIdentity.tsx
They need to be updated to:
import ErrorMessage from 'components/messages/ErrorMessage'Also, could you update the "How did you test this code?" section in the PR description? Even for mechanical refactors, a quick smoke test goes a long way. Something like:
Ran
npm run dev, opened a few pages that display error states (e.g. submitting an invalid form), and confirmed the error messages render correctly.
Running the dev server locally would have caught the missing imports straight away — good habit that saves review cycles!
Could you push a fix for those four imports and update the test section? Happy to help if you get stuck!
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #6891
Please describe.
deleted the component/ErrrorMessage.js file which was a legacy duplicate of current component/messages/ErrorMessage.tsx , the legacy duplicate was imported into 34 files , all of them now use the latest tsx version.
How did you test this code?
not required
Please describe.
not required