Skip to content

Add remark-cjk-friendly(-gfm-strikethrough) to plugins.md#1422

Merged
wooorm merged 1 commit intoremarkjs:mainfrom
tats-u:my-plugins
Apr 10, 2025
Merged

Add remark-cjk-friendly(-gfm-strikethrough) to plugins.md#1422
wooorm merged 1 commit intoremarkjs:mainfrom
tats-u:my-plugins

Conversation

@tats-u
Copy link
Copy Markdown
Contributor

@tats-u tats-u commented Mar 15, 2025

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and discussions and couldn’t find anything or linked relevant results below
  • I made sure the docs are up to date
  • I included tests (or that’s not needed) (N/A)

Description of changes

Adds the following plugins to plugins.md:

CommonMark's emphasis specification is unfriendly to CJK languages. (c.f. #1107 and #1288). These plugins mitigate this problem and are served as a public test for a new Commonmark specification candidate.

repo: https://github.com/tats-u/markdown-cjk-friendly/

@github-actions github-actions Bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Mar 15, 2025
@tats-u tats-u changed the title Introduce remark-cjk-friendly(-gfm-strikethrough) in plugins.md Add remark-cjk-friendly(-gfm-strikethrough) to plugins.md Mar 15, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (a185821) to head (280cde6).
Report is 42 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1422   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          142       138    -4     
=========================================
- Hits           142       138    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ChristianMurphy
Copy link
Copy Markdown
Member

Thanks for sharing @tats-u! 🙇
A few notes:

  1. it looks like the types are intended to be built https://github.com/tats-u/markdown-cjk-friendly/blob/b9c2ac657a0441ca47938a565491abe69b777e81/packages/remark-cjk-friendly-gfm-strikethrough/package.json#L5-L7 but didn't actually make it to npm https://www.npmjs.com/package/remark-cjk-friendly-gfm-strikethrough (in a way npm understands)
  2. The module type should probably be node16 or nodenext https://github.com/tats-u/markdown-cjk-friendly/blob/b9c2ac657a0441ca47938a565491abe69b777e81/packages/remark-cjk-friendly-gfm-strikethrough/tsconfig.json#L3-L4 source https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-node18-nodenext
  3. I'd recommend against bundling as part of build https://github.com/tats-u/markdown-cjk-friendly/blob/b9c2ac657a0441ca47938a565491abe69b777e81/packages/remark-cjk-friendly-gfm-strikethrough/package.json#L32 this introduces indirection that makes the code more complex, less performant, and harder to debug. Ship code that has the types removed, but otherwise is as close as possible to the source.

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Mar 22, 2025

I adopted it only for libraries because the language server recommends "moduleResolution": "nodenext" for libraries and "bundlers" only for applications. "module": "NodeNext" is not compatible with "moduleResolution": "bundler" so I have avoided it.

There are .d.ts files in NPM:

https://www.npmjs.com/package/remark-cjk-friendly?activeTab=code / https://www.npmjs.com/package/remark-cjk-friendly-gfm-strikethrough?activeTab=code

There is only single .js file in both remark plugin packages. https://www.npmjs.com/package/remark-cjk-friendly?activeTab=code / https://www.npmjs.com/package/remark-cjk-friendly-gfm-strikethrough?activeTab=code

makes the code more complex, less performant, and harder to debug

The structure of the code is mostly kept except for the long prefix __WEBPACK_EXTERNAL_MODULE_ like The Nile or The Great Wall. If you hate it, I recommend you to file an issue in Rslib.

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Mar 22, 2025

My remark plugins only register corresponding micromark extensions as remark-gfm does.

@ChristianMurphy
Copy link
Copy Markdown
Member

ChristianMurphy commented Mar 22, 2025

I adopted it only for libraries

Your code is a library.
It is published to npm, not as a web application.
Node16 or nodenext is correct.
To quote the typescript docs

A common misconception is that node16—nodenext only emit ES modules. In reality, these modes describe versions of Node.js that support ES modules, not just projects that use ES modules. Both ESM and CommonJS emit are supported, based on the detected module format of each file. Because they are the only module options that reflect the complexities of Node.js’s dual module system, they are the only correct module options for all apps and libraries that are intended to run in Node.js v12 or later, whether they use ES modules or not.

https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-node18-nodenext

There are .d.ts files in NPM:

NPM does not recognize them, it expects a type property https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package
Which means your module is flagged as being untyped regardless of having .d.ts files

If you hate it, I recommend you to file an issue in Rslib.

They already support it with bundle: false. https://lib.rsbuild.dev/config/lib/bundle#bundle-false
This isn't an rslib issue, it is configuration issue in your code.

@ChristianMurphy
Copy link
Copy Markdown
Member

My remark plugins only register corresponding micromark extensions as remark-gfm does.

I'm commenting on the remark plugins, because you are asking the remark team to recommend them.
That said my comments also apply to the micromark and markdown it code in the same repo.

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Mar 22, 2025

It is published to npm, not as a web application.

My repo contains demo applications for Docusaurus, Rspack, and Astro. Their tsconfig has not been changed.

Important

tsconfig is not bundles into my packages. It is used only when built by Rslib.

tats-u/markdown-cjk-friendly@968978b

They already support it with bundle: false.

I tried it but it didn't erase the super long variable prefix. It doesn't have effect if the number of .ts file is just one.

@ChristianMurphy
Copy link
Copy Markdown
Member

My repo contains demo applications for Docusaurus, Rspack, and Astro. Their tsconfig has not been changed.

  1. Everything used with node12+ should use the correct module specifier according to the typescript team themselves. Feel free to log bugs with other tools if they don't follow conventional.
  2. The code is published to npm should follow library conventional, not app ones.

I tried it but it didn't erase the super long variable prefix. It doesn't have effect if the number of .ts file is just one.

You may not need rslib at all.
Right now it's only acting as an unnecessary wrapper around the typescript compiler.
You could just call the typescript compiler directly.

@ChristianMurphy
Copy link
Copy Markdown
Member

tsconfig is not bundles into my packages. It is used only when built by Rslib.

Right, when it is used by the typescript compiler it is producing a potentially wrong output.

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Mar 22, 2025

it expects a type property https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html#including-declarations-in-your-npm-package

Prettier doesn't have the top-level "types" but recognized as typed because "types" exists in every entry in "export". My package.json adopts the same strategy because my packages don't need to support Node versions that don't recognize "export". However, since only micromark-extension-cjk-friendly-gfm-friendly, which has the top-level "types", has the TS badge, so I chose to add the top-level "types" to every maintained package.

https://www.npmjs.com/package/prettier?activeTab=code

markdown it code

markdown-it plugins are out of the unified ecosystem. I won't turn off bundling it. (in the first place bundle: false doesn't have effect in ESM output there)

Right now it's only acting as an unnecessary wrapper around the typescript compiler.

If I didn't use v flag of the regex, I would have to transpile the cutting edge syntax using Rslib or similar software.

You may not need rslib at all.

Many libraries use tsup and I recognize Rslib plays the same role. I've started trying it, and the output looks better. However, the build time seems to be longer than Rslib.

@ChristianMurphy
Copy link
Copy Markdown
Member

Prettier doesn't have the top-level "types" but recognized as typed because "types" exists in every entry in "export"

That works for actual code.
But for npm to show the typescript badge, it's not enough.
I think it shows for them because they have index.d.ts at the root level.

I chose to add the top-level "types" to every maintained package.

Sounds good.

the cutting edge syntax using Rslib or similar software.

Or you could write valid JavaScript that works in Node LTS, which is already fairly modern and skip the transpilers. 🙂

Many libraries use tsup and I recognize Rslib plays the same role. I've started trying it, but the output looks better. However, the build time seems to be longer than Rslib.

Okay, you may not need tsup either

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Mar 23, 2025

I republished packages using tsup.
It was very easy to configure without being suffered from tsconfig.json.
Could you examine the content of published packages again? I believe it got closer to what you want it to be.
Also all of my packages were able to get the [TS] badge thanks to your advice.

If you prefer UNPKG (1 page per file):

Or you could write valid JavaScript that works in Node LTS, which is already fairly modern and skip the transpilers. 🙂

I want transpilers to be an "insurance".

@ChristianMurphy
Copy link
Copy Markdown
Member

ChristianMurphy commented Mar 23, 2025

LGTM

I want transpilers to be an "insurance".

I hear you 🙂
You have a library version set, which will validate your source sticks to features in that version https://github.com/tats-u/markdown-cjk-friendly/blob/5a3c70679e2954d846517f93ee4376df8678b481/packages/remark-cjk-friendly/tsconfig.json#L3
Current LTS of Node supports es2022

"lib": ["es2022"],

And ts gives some insurance that the code will work as is

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Mar 23, 2025

I know Node 16 is EOL, but started adopting the Prettier's policy and setting the minimum version as that of micromark:

https://github.com/prettier/prettier/wiki/The-policy-to-drop-Node.js-version

https://github.com/micromark/micromark/blob/3fae15528f69dfaf2a8865a7f7d92bfb4abd7bc9/readme.md?plain=1#L104

ES2021 is the version said to be the supported version by Node 16.

https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping

You have a library version set, which will validate your source sticks to features in that version https://github.com/tats-u/markdown-cjk-friendly/blob/5a3c70679e2954d846517f93ee4376df8678b481/packages/remark-cjk-friendly/tsconfig.json#L3

Tsc doesn't provide a transpilation of the v flag in regex, so I've not touched the "lib" entry until a new feature is necessary for my libraries.

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Apr 3, 2025

Do you have anything else that you still concerned about?

Comment thread doc/plugins.md Outdated
@tats-u tats-u requested a review from wooorm April 3, 2025 14:53
@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Apr 3, 2025

Is the nested entry OK? -gfm-strikethrough is just a subpackage of remark-cjk-friendly and used together with it.

@wooorm
Copy link
Copy Markdown
Member

wooorm commented Apr 9, 2025

No, thank you. Please keep with the style that is in place. Please use short descriptions. In-depth information can go in your documentation.

@tats-u
Copy link
Copy Markdown
Contributor Author

tats-u commented Apr 9, 2025

Please use short descriptions.

Do you still think "make emphasis in CJK languages ​​more reliably recognized" is too long?

How can I make it shorter than "recognize emphasis in CJK languages more reliably"?

@wooorm wooorm merged commit 2854525 into remarkjs:main Apr 10, 2025
6 checks passed
@github-actions

This comment has been minimized.

@wooorm wooorm added the 💪 phase/solved Post is done label Apr 10, 2025
@github-actions github-actions Bot removed the 🤞 phase/open Post is being triaged manually label Apr 10, 2025
@wooorm
Copy link
Copy Markdown
Member

wooorm commented Apr 10, 2025

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💪 phase/solved Post is done

Development

Successfully merging this pull request may close these issues.

3 participants