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

imprv: Update dependencies due to unified package upgrade #8995

Conversation

reiji-h
Copy link
Contributor

@reiji-h reiji-h commented Jul 26, 2024

Summary

  • unified のパッケージアップグレードに伴うエラーの解消

Task

Note

  • hast-util-sanitize/lib の lint error は後続タスクで解決予定(これ以外の lint error はおそらく無い)
  • remark-growi-directive を使用しているパッケージ等が any を使っているものは後続タスクで解決予定
  • ReactMarkdown の Component の型は Memo 化されたコンポーネントを受け付けないので as typeof で型を消している。(参考)
  • ReactMarkdown は 9.0 より inlineCode
code

の判別方法を提供しなくなったので、 remark-plugins/codeblock.ts で無理やり判別している。

Copy link

changeset-bot bot commented Jul 26, 2024

⚠️ No Changeset found

Latest commit: 8165837

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@@ -5,7 +5,7 @@ import { findAfter } from 'unist-util-find-after';
import { visit } from 'unist-util-visit';


function wrapWithSection(parentNode: Parent, startElem: Node, endElem: Node | null, isDarkMode?: boolean): void {
function wrapWithSection(parentNode: Parent, startElem: Node, endElem: Node | undefined | null, isDarkMode?: boolean): void {
Copy link
Member

Choose a reason for hiding this comment

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

endElem?: Node | null で書けそう?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

書き換えました。

type Lang = 'drawio';

function isDrawioBlock(lang: unknown): lang is Lang {
return /^drawio$/.test(lang as string);
function isDrawioBlock(lang: string | undefined | null): lang is Lang {
Copy link
Member

Choose a reason for hiding this comment

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

lang?: string | null で書けそう?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

書き換えました。

Copy link
Member

@yuki-takei yuki-takei left a comment

Choose a reason for hiding this comment

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

  • import 文の as 利用は OK
  • memolized component の as typeof 利用も OK
  • 処理ステートメント中の as (たとえば (node as Code) など) は、こんなに多用しないといけないのはなんか釈然としない
    • type guard とかないのか?
    • ないなら作ってはどうか?as Parent や as Code のケースが多そうなのである程度共通化もできそう

// if inlineCode, properties.inline exists.
if (element.properties?.inline != null) {
element.properties.inline = true;
addClassToProperties(element.properties, 'inline');
Copy link
Member

Choose a reason for hiding this comment

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

sanitize の設定を見直すことで class には入れなくても property として扱えると思う

getCommonSanitizeOption を呼んで rehypeSanitizePlugin を作っているところで code 用の設定も入れ込む

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sanitize を設定し、property から inline を判断するようにしました。

@yuki-takei yuki-takei merged commit 7d0a6fa into imprv/148445-upgrade-remark-growi-directive Aug 26, 2024
4 of 5 checks passed
@yuki-takei yuki-takei deleted the imprv/148445-15079-update-dependencies branch August 26, 2024 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants