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

Deprecate MarkdownEditor, MarkdownViewer, InlineAutocomplete, and related code #4027

Merged
merged 12 commits into from
Dec 13, 2023

Conversation

iansan5653
Copy link
Contributor

@iansan5653 iansan5653 commented Dec 7, 2023

Per #3604, we will migrate the following draft/experimental APIs from primer/react to the internal GitHub codebase. Since drafts are not semvered we technically don't have to deprecate them, but I think it's still probably a good idea to follow a deprecation process rather than having them just disappear. So this PR deprecates the components, and then in v37 we can ultimately delete the code. This would be an aggressive timeline for other APIs, but again these are all drafts:

  • MarkdownEditor
  • MardownViewer
  • InlineAutocomplete
  • hooks/useCombobox
  • hooks/useDynamicTextareaHeight
  • hooks/useIgnoreKeyboardActionsWhileComposing
  • hooks/useSafeAsyncCallback
  • hooks/useSyntheticChange
  • hooks/useUnifiedFileSelect, useClickFileSelect, useDropFileSelect, usePasteFileSelect

While some internal APIs will be migrated as well, I've focused on public APIs here.

Side note: I also moved InlineAutocomplete into drafts where it belongs. Not sure how it escaped 🤷.

Changelog

New

Changed

  • Deprecated MarkdownEditor, MardownViewer, InlineAutocomplete, and related hooks

Removed

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan

Testing & Reviewing

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@iansan5653 iansan5653 requested review from a team and pksjce December 7, 2023 17:11
Copy link

changeset-bot bot commented Dec 7, 2023

🦋 Changeset detected

Latest commit: 42016cf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Dec 7, 2023

size-limit report 📦

Path Size
dist/browser.esm.js 105.54 KB (0%)
dist/browser.umd.js 106.13 KB (0%)

@siddharthkp siddharthkp self-requested a review December 7, 2023 17:15
@joshblack
Copy link
Member

@iansan5653 just wanted to ask, is usage over in dotcom already migrated over to use the internal version of this component or would it be helpful to add linting to help folks move over to the new package?

@siddharthkp siddharthkp requested review from broccolinisoup and siddharthkp and removed request for siddharthkp December 7, 2023 17:21
@iansan5653
Copy link
Contributor Author

iansan5653 commented Dec 7, 2023

usage over in dotcom already migrated over to use the internal version of this component or would it be helpful to add linting to help folks move over to the new package?

Not yet. I haven't actually copied the code over to dotcom, but when I do so I will add linting accordingly. I have already added rules to lint specifically against importing @primer/react/drafts/MarkdownEditor since we have CommentBox in dotcom (inflight).

Copy link
Contributor

@mperrotti mperrotti left a comment

Choose a reason for hiding this comment

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

Assuming we don't want to move these to the deprecated package yet, this looks correct to me.

@iansan5653
Copy link
Contributor Author

Assuming we don't want to move these to the deprecated package yet, this looks correct to me.

Not sure what we would do here, since they are already in the drafts package. I'm open to moving them if you want.

@iansan5653 iansan5653 added this pull request to the merge queue Dec 13, 2023
Merged via the queue into main with commit 49f585f Dec 13, 2023
29 checks passed
@iansan5653 iansan5653 deleted the deprecate-markdowneditor branch December 13, 2023 18:06
This was referenced Dec 13, 2023
siddharthkp added a commit that referenced this pull request Nov 1, 2024
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.

3 participants