Add remark-flexible-code-titles to list of plugins

[talatkuyuk]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

Added a new plugin remark-flexible-code-titles to list of plugins

https://github.com/ipikuka/remark-flexible-code-titles

[codecov-commenter]

Codecov Report

Merging #1108 (b961049) into main (bec44aa) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #1108   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          906       906           
=========================================
  Hits           906       906           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bec44aa...b961049. Read the comment docs.

[remcohaszing]

Thanks!

[ChristianMurphy]

Thanks for sharing @talatkuyuk!

Looks good, a few tips for the plugin.

1. For NodeNext or Node16 module resolution export types needs to be set https://github.com/ipikuka/remark-flexible-code-titles/blob/688b55ac891a544a4f9485849bafb7f9bc3738cb/package.json#L6-L11 see https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing for more on this
2. mdast and unist appear to be part of the public interface, if so it would be advisable to include their @types/ package in dependencies https://github.com/ipikuka/remark-flexible-code-titles/blob/688b55ac891a544a4f9485849bafb7f9bc3738cb/package.json#L54
3. You could probably simplify the typings by noting that the plugin accepts an mdast Root node, see https://github.com/remarkjs/remark-validate-links/blob/38da293cf84ff3cfda8caf8f45321536cc58fa94/lib/index.js#L54 for an example, when that type is set type narrowing like isCodeNode (https://github.com/ipikuka/remark-flexible-code-titles/blob/688b55ac891a544a4f9485849bafb7f9bc3738cb/src/index.ts#L69-L71) could be handled by the visit function itself
4. For debugging purposes, it can be useful to make most functions named functions, rather than arrow functions assigned to a variable

[talatkuyuk]

Thank you @ChristianMurphy for quick review and tips.

Number-1

For NodeNext or Node16 module resolution export types needs to be set.

In the dist/esmand dist/cjs directories in the package, the index.d.ts file are colocated with the index.js file. In that cases no need to set export types explicitly. The doc you refer says:

...it will look for a colocated declaration file. If you need to point to a different location for your type declarations, you can add a "types" import condition.

Number-2

mdast and unist appear to be part of the public interface, if so it would be advisable to include their @types/ package in dependencies

They are not a part of the public interface (only Plugin itself). I suppose the "@types/mdast": "^3.0.10" and "@types/unist": "^2.0.6" are better to stay in devDependencies.

Number-3

You could probably simplify the typings by noting that the plugin accepts an mdast Root node.

I added as you pointed out.

export const plugin: Plugin<[CodeTitleOptions?], Root>

but I receive typing errors:

Type 'Transformer<Node<Data>, Node<Data>>' is not assignable to type 'Transformer<Root, Root>'.
      Property 'children' is missing in type 'Node<Data>' but required in type 'Root'.

I tried to fix it but couldn't figure it out. Is there any plugin that uses the types Plugin, Transformer, Visitor and narrowed the Node to a specific node like Code, Paragraph, Literal etc. by visit function itself and written in fully typescript? I need support for it.

I needed the function isCodeNode in order to narrow the Node<Data> into Code to get/set the node.lang or node.meta without typing problem.

Number-4

For debugging purposes, it can be useful to make most functions named functions, rather than arrow functions assigned to a variable.

Arrow functions are anonymous, which means that they are not named. This is a problem if you want to debug because you will not be able to trace the name of the function. To solve this, you could use a feature introduced in ES2015-ES6 that is the function name inference. JavaScript can determine the arrow function name from its syntactic position: from the variable name that holds the function object. it is a good practice to use function name inference to name the arrow functions if you want to debug your code. Quoted from this article

Another stackoverflow reference for it.

Thank you for your review again, I have learnt a lot while digging them.

[talatkuyuk]

Hi @wooorm, I made the change that you requested.

[talatkuyuk]

Hi @ChristianMurphy,

About exporting types explicitly, I found the option in the tsconfig.json namely declarationDir that copies theindex.d.ts to the specified directory, in my case "./dist/types". Now, the index.d.ts is in the different directory from index.js file. So I set the export types explicitly in the package.json. Just for your information.

[talatkuyuk]

Hi @ChristianMurphy,

About simplifying typings, I fixed the typescript error that I receive. Now, the plugin accepts the mdast Root node, and type narrowing to "Code" is handled by the visit function itself. I removed the isCodeNode. Thanks for the tip.

[ChristianMurphy]

Thanks @talatkuyuk!

Appreciate the updates and for sharing the plugin with the community.
This LGTM 👍

Some, completely optional, follow up ideas/suggestions.

1. in tsconfig.json when module: node16 is set: moduleResolution, esModuleInterop, and importHelpers can be omitted (and probably should be) https://github.com/ipikuka/remark-flexible-code-titles/blob/fc0cf0b262911846aded817adc9b244f1743704f/tsconfig.json#L6-L9
2. with mdast.Root being used now, @types/mdast should probably move to dependencies (see: https://github.com/ipikuka/remark-flexible-code-titles/blob/fc0cf0b262911846aded817adc9b244f1743704f/src/index.ts#L80 and https://github.com/ipikuka/remark-flexible-code-titles/blob/fc0cf0b262911846aded817adc9b244f1743704f/package.json#L52)
3. In package.json for dependencies use a wide ^ range where possible. Pinning specific versions of dependencies can cause version conflicts and duplication in the node_modules folder. For example "unist-util-visit": "^4.1.2" could be widened to "unist-util-visit": "^4.0.0" to allow for further deduplication.
4. TypeScript has a different/looser interpretation of SemVer, they can introduce breaking changes for libraries in minor releases. Depending on how you want to handle that, you could use a ~ to lock down a specific minor release in tests, or keep it ^ with the expectation that it may break every now and again https://github.com/ipikuka/remark-flexible-code-titles/blob/fc0cf0b262911846aded817adc9b244f1743704f/package.json#L70
5. it looks like it could be safe to add sideEffects: false to package.json which helps bundlers like webpack with tree shaking (https://webpack.js.org/guides/tree-shaking/#mark-the-file-as-side-effect-free)
6. Unless you specifically need a script to run during the install phase (I don't see any currently) I'd recommend setting ignore-scripts=true in .npmrc to limit supply chain risks from upstream packages having unnecessary life cycle scripts.

[talatkuyuk]

Hi @ChristianMurphy, I implemented all your suggestions in version 1.1.1 . I have learnt a lot, thanks again.

[wooorm]

Thank you Talat! Glad to hear Christian’s tips are useful! :)

I also see you are from Türkiye. I am so sorry for the terrible things happening there, and in Syria, right now. I hope you and your loved ones are okay!

[talatkuyuk]

Hi Titus,

Thank you very much for your message with good wishes.

Me and my family are fine. We live quite a distance from the earthquake zone. We have no loss of life from my own and my wife's relatives. However, some relatives of our friends lost their lives and there were those whose houses were destroyed.

The dimensions of the earthquake disaster are huge. More than 100,000 lives are thought to have been lost, and the vast majority are still under the rubble. We are very sad. May God not let any nation experience such a calamity.