BREAKING: remove DeprecatedModal component

pedroalves0 commented 10 months ago

Short description

The DeprecatedModal component is built on top of the @reach/dialog module, which is no longer maintained and lacks support for react v18. When installing Reactist in a project using the latest React version, similar warnings to the following are thrown:

npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: @reach/dialog@0.16.2
npm WARN Found: react@18.2.0
npm WARN node_modules/react
npm WARN   react@"18.2.0" from the root project
npm WARN   117 more (@contentful/rich-text-react-renderer, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer react@"^16.8.0 || 17.x" from @reach/dialog@0.16.2
npm WARN node_modules/@doist/reactist/node_modules/@reach/dialog
npm WARN   @reach/dialog@"^0.16.0" from @doist/reactist@21.3.0
npm WARN   node_modules/@doist/reactist

As the component has been marked for deprecated for a long time now, it's reasonable to assume that it can be removed (alongside @reach/dialog) without major impact to consumers.

PR Checklist

[ ] Added tests for bugs / new features
[ ] Updated docs (storybooks, readme)
[x] Executed npm run validate and made sure no errors / warnings were shown
[x] Described changes in CHANGELOG.md
[x] Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
[ ] Reviewed and approved Chromatic visual regression tests in CI

Versioning

Major version bump to v22.0.0, as the removal of an exported component is a breaking change.

pedroalves0 commented 10 months ago

I'm aware that Twist still uses it @rfgamaral , but given that development on Twist is somewhat stalled, I think it's a fair compromise that we'll move on with modernizing Reactist. Given your pushback, let's expand the discussion to the team; I'll create a thread.

rfgamaral commented 10 months ago

Given your pushback, let's expand the discussion to the team; I'll create a thread.

It's not a pushback per se, I'm just concerned that when Twist development resumes, we'll have a harder time doing so, because we might need to upgrade Reactist to the latest version for small changes/improvements, but won't be able to without first dealing with the removal DeprecatedModal that is being heavily used by it.

I think it's a fair compromise that we'll move on with modernizing Reactist.

Maybe you're right, but I just wanted to bring that to our attention.

pedroalves0 commented 10 months ago

Update here, our internal discussion concluded that the effort to migrate the deprecated modals on Twist is relatively small. Thus, I'm going ahead with the merge of this PR.

Doist / reactist