Skip to content

Major version dependency upgrades, pt. V#4124

Merged
thompsongl merged 20 commits intoelastic:masterfrom
thompsongl:3969-ptv
Oct 19, 2020
Merged

Major version dependency upgrades, pt. V#4124
thompsongl merged 20 commits intoelastic:masterfrom
thompsongl:3969-ptv

Conversation

@thompsongl
Copy link
Copy Markdown
Contributor

@thompsongl thompsongl commented Oct 8, 2020 •
edited
Loading

Summary

Partial resolution of #3969: EuiMarkdownEditor dependencies

  • react-dropzone
  • Misc. rehype-
  • Misc. remark-
  • unified

Most major version changes were due to the introduction of types (:tada:), but unified did have an API change: the result of .process[Sync]() is now stored in result if an object or contents if a string. See relevant markdown editor changes accounting for both.

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in Chrome, Safari, Edge, and Firefox
- [ ] Props have proper autodocs
- [ ] Added documentation
- [ ] Checked Code Sandbox works for the any docs examples
- [ ] Added or updated jest tests

  • Checked for breaking changes and labeled appropriately

- [ ] Checked for accessibility including keyboard-only and screenreader modes

  • A changelog entry exists and is marked appropriately

@thompsongl thompsongl added the dependencies PRs that update a dependency file label Oct 8, 2020
@thompsongl thompsongl mentioned this pull request Oct 8, 2020
Closed
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Oct 8, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_4124/

chandlerprall
chandlerprall approved these changes Oct 12, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

One typo correction & needs a breaking changelog entry. Tested the markdown editor in the CI's docs build, would be really good to try this out in Kibana to get an expectation of how much effort this will (or won't) be to update to.

@thompsongl
Copy link
Copy Markdown
Contributor Author

thompsongl commented Oct 12, 2020

@chandlerprall I didn't expect this to be breaking. Is it something with the plugin API?

@chandlerprall
Copy link
Copy Markdown
Contributor

chandlerprall commented Oct 12, 2020

@chandlerprall I didn't expect this to be breaking. Is it something with the plugin API?

That was based off the dependencies list changing, but those would should be transparently handled by npm/yarn and aren't breaking changes. So yeah, just calling out the upgrades in a changelog entry should be good 👍

@thompsongl @chandlerprall
Update src/components/markdown_editor/markdown_format.tsx
da15510 
Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
@thompsongl thompsongl marked this pull request as ready for review October 12, 2020 21:28
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Oct 12, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_4124/

@thompsongl
Copy link
Copy Markdown
Contributor Author

thompsongl commented Oct 12, 2020

Checking on the upgrade path in Kibana. Looks like maybe some more nohoist changes and some types to fix. Will have a resolution accounted for before merging.

@thompsongl
Copy link
Copy Markdown
Contributor Author

thompsongl commented Oct 13, 2020

Will have a resolution accounted for before merging.

Need to add "@elastic/eui/remark-parse" and "@elastic/eui/unified" to Kibana's nohoist list.

Type updates are trickier: the return type of getDefaultEuiMarkdownProcessingPlugins no longer matches the expected type of the processingPluginList prop. cc// @chandlerprall

@thompsongl
Copy link
Copy Markdown
Contributor Author

thompsongl commented Oct 15, 2020 •
edited
Loading

Upgrade path in Kibana is now known. Also confirmed that the plugins written in Kibana still work

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Oct 15, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_4124/

chandlerprall
chandlerprall approved these changes Oct 16, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM!

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Oct 16, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_4124/

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Oct 19, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_4124/

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Oct 19, 2020

Preview documentation changes for this PR: https://eui.elastic.co/pr_4124/

@thompsongl thompsongl merged commit b99554e into elastic:master Oct 19, 2020
@thompsongl thompsongl mentioned this pull request Oct 22, 2020
Merged
kshitij86 added a commit to kshitij86/eui that referenced this pull request Nov 29, 2020
@kshitij86 @chandlerprall
Major version dependency upgrades, pt. V (elastic#4124)
b192303 
* eslint; prettier

* react-datepicker eslint

* prettier autofix

* more eslint

* CL

* Revert "CL"

This reverts commit 745d9a0.

* markdown deps

* contents | result update

* Update src/components/markdown_editor/markdown_format.tsx

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>

* CL

* update remark2rehype types

* CL

* Update CHANGELOG.md

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@chandlerprall chandlerprall chandlerprall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

dependencies PRs that update a dependency file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thompsongl @kibanamachine @chandlerprall