feat: upgrade react-markdown #681
Conversation
upgrade required for the following reasons 1. dependabot reported a security issue with a package (trim) the older version of react-markdown was using. Note that the trim packages is still referenced by eslint tooling. This will be dealt with in a separate PR. 2. dependency on path-browserify and process-browser can be removed. The replacement was not straight forward as the mechanism for providing alternative rendering of components significantly changed. The Markdown component was refactored into its own component as react-markdown was being used in two places. It turns out that this was not necessary as the 2nd usage needs to use the raw react-markdown component at least for. The refactor was kept as it is possible the markdown component may be useful elsewhere. The original implementation had a dependency on specKey which has now been removed. In addition the following changes have been done: 1. Links to looker documentation in an extension now supported. 2. List components render correctly. 3. Links that have markup from search now work.
APIX Tests 1 files 72 suites 2m 54s ⏱️ Results for commit 654552f. |
| const handleClick = (e: BaseSyntheticEvent) => { | ||
| e.preventDefault() | ||
| if (linkClickHandler) { | ||
| // Could be a <mark> tag wrapped by an anchor |
There was a problem hiding this comment.
I like this new file a lot.
Similar to how anchors have a special case,
code blobs have a currently uncaught special case. It looks like your change makes this easier to address:
There was a problem hiding this comment.
Agreed. I think this file belongs in code-editor rather than here, so we can reuse it outside of APIX. I also hope to convert triple tick code blocks to use code display
|
@josephaxisa - that's not an issue introduced by my change. Also happens in the current marketplace version. Suspect it's going to be tricky to solve as I think code is similar to pre in that it ignores child elements. Create a separate issue is my recommendation. Marketplace APIX version below
|
APIX Tests 1 files 72 suites 2m 8s ⏱️ Results for commit f8df883. |
APIX Tests 1 files 72 suites 2m 33s ⏱️ Results for commit ccb02e3. |
| @@ -70,8 +52,9 @@ export const remapHashURL = (specKey: string, url: string) => | |||
| /** | |||
| * Clean urls from mark tags and remap tag/method hashbang urls to match the MethodScene route | |||
| * @param specKey A string to identify the spec in the URL | |||
| * @param url | |||
| * @param url - url | |||
There was a problem hiding this comment.
What does this comment mean?
| import { renderWithSearch } from '../../test-utils' | ||
| import { Markdown } from './Markdown' | ||
|
|
||
| describe('Markdown', () => { |
| const handleClick = (e: BaseSyntheticEvent) => { | ||
| e.preventDefault() | ||
| if (linkClickHandler) { | ||
| // Could be a <mark> tag wrapped by an anchor |
There was a problem hiding this comment.
Agreed. I think this file belongs in code-editor rather than here, so we can reuse it outside of APIX. I also hope to convert triple tick code blocks to use code display
This comment has been minimized.
This comment has been minimized.
upgrade required for the following reasons
The replacement was not straight forward as the mechanism for providing alternative rendering of components significantly changed. The Markdown component was refactored into its own component as react-markdown was being used in two places. It turns out that this was not necessary as the 2nd usage needs to use the raw react-markdown component at least for. The refactor was kept as it is possible the markdown component may be useful elsewhere. The original implementation had a dependency on specKey which has now been removed.
In addition the following changes have been done: