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

Type hints seem to be incorrect #29

Closed
kamoshi opened this issue Oct 28, 2023 · 14 comments
Closed

Type hints seem to be incorrect #29

kamoshi opened this issue Oct 28, 2023 · 14 comments

Comments

@kamoshi
Copy link

kamoshi commented Oct 28, 2023

When trying to use this plugin in Astro 3.4 I see the following TypeScript error:

Diagnostics:
1. Type 'Plugin<[(RemarkEmojiOptions | undefined)?]>' is not assignable to type 'string | [string, any] | RemarkPlugin<any[]> | [RemarkPlugin<any[]>, any]'.
     Type 'Plugin<[(RemarkEmojiOptions | undefined)?]>' is not assignable to type 'RemarkPlugin<any[]>'.
       The 'this' types of each signature are incompatible.
         Type 'Processor<void, void, void, void>' is missing the following properties from type 'Processor<undefined, undefined, undefined, undefined, undefined>': compiler, freezeIndex, frozen, namespace, and 3 more. [2322]

Astro expects an array of the following type:
https://github.com/withastro/astro/blob/5fed432b0c3c84542a3d1b2952d183e9cbe3fa0e/packages/markdown/remark/src/types.ts#L28

This plugin does work correct however.

@rhysd
Copy link
Owner

rhysd commented Oct 28, 2023

At least please provide remark packages versions, TypeScript version, your tsconfig.

remark bumped major version last month. It may affect on type definitions.

@kamoshi
Copy link
Author

kamoshi commented Oct 28, 2023

Sure

  • tsserver: 4.0.0
  • TypeScript: 5.2

I use astro 3.4, which in turn depends on:

  • "unified": "^10.1.2"
  • "@types/mdast": "^3.0.12"

The full package.json can be seen here

The type expected by Astro is:

export type RemarkPlugin<PluginParameters extends any[] = any[]> = unified.Plugin<
	PluginParameters,
	mdast.Root
>;

export type RemarkPlugins = (string | [string, any] | RemarkPlugin | [RemarkPlugin, any])[];

Problem occurs when passing remarkEmoji into an array which has the type RemarkPlugins:
image

@rhysd
Copy link
Owner

rhysd commented Oct 28, 2023

Could you share the exact version of remark-emoji package in your local?

If okay, it is helpful to dump all the dependendcies by npm list --all and paste it here.

@kamoshi
Copy link
Author

kamoshi commented Oct 28, 2023

remark-emoji 4.0.0

Here is the output of pnpm ls --depth Infinity:
out.txt

@rhysd rhysd closed this as completed in 5082772 Oct 29, 2023
@rhysd
Copy link
Owner

rhysd commented Oct 29, 2023

I improved type definitions in this package and released v4.0.1. Could you try the latest version?

https://github.com/rhysd/remark-emoji/releases/tag/v4.0.1

@goingtogogo
Copy link

hey hey @rhysd!
I'm sorry for the lack of details. dependabot brought a PR, and it appears that it started working but not completely😔

(using remark-emoji with next-mdx-remote, @mdx-js)

Снимок экрана 2023-11-01 в 09 31 34

@kamoshi
Copy link
Author

kamoshi commented Nov 1, 2023

I still have the same problem:
image

I checked one of the official plugins how they are using types, and it turns out that there might be some problem with TypeScript and remark types in general. This seems relevant:
https://github.com/remarkjs/remark-math/blob/main/packages/remark-math/lib/index.js#L26-L30

Maybe @wooorm would know more?

@wooorm
Copy link
Collaborator

wooorm commented Nov 1, 2023

The types are fine. The problem is that there are different types (because mismatched dependencies). And that’s how TypeScript works: adding one field in a type somewhere is breaking. Which is why I did breaking changes everywhere.

The solution is to update all your tools, not just some.
So update MDX too.
And if next-mdx-remote is not yet updated, raise a PR to use MDX 3 there.

This is how TS works, it warns when things might not work. You can add a @ts-expect-error when you know it works. And otherwise update all your dependencies.


This seems relevant:

Nope, that’s a solution to a bug with how TS generates .d.ts files; unrelated to this!

@wooorm
Copy link
Collaborator

wooorm commented Nov 1, 2023

Reading through the types in remark-emoji: they look perfect!

@kamoshi
Copy link
Author

kamoshi commented Nov 1, 2023

It seems the problem isn't here then, nevertheless thanks and sorry for bugging you 😅

@rhysd
Copy link
Owner

rhysd commented Nov 4, 2023

I missed this conversation since this issue was closed.

@wooorm Thank you for your follow up.

@tdkn
Copy link

tdkn commented Nov 24, 2023

@rhysd
First of all, thanks for this useful plugin.

I'm using remark-emoji with contentlayer.
After updating from version 4.0.0 to 4.0.1, I've encountered the following error:

Type 'Plugin<[(RemarkEmojiOptions | null | undefined)?], Root>' is not assignable to type 'Pluggable<any[]>'.
  Type 'Plugin<[(RemarkEmojiOptions | null | undefined)?], Root>' is not assignable to type 'Plugin<any[], any, any>'.
    The 'this' types of each signature are incompatible.
      Type 'Processor<void, any, void, void> | Processor<void, any, any, any> | Processor<any, any, void, void> | Processor<void, void, void, void>' is not assignable to type 'Processor<undefined, undefined, undefined, undefined, undefined>'.
        Type 'Processor<void, any, void, void>' is missing the following properties from type 'Processor<undefined, undefined, undefined, undefined, undefined>': compiler, freezeIndex, frozen, namespace, and 3 more.

reproduction: tdkn/blog#447

I would appreciate any advice on how to resolve this issue.

@wooorm
Copy link
Collaborator

wooorm commented Nov 24, 2023

contentlayer should update things

@tdkn
Copy link

tdkn commented Nov 24, 2023

@wooorm
Thank you for the quick response.
I will try to investigate where to fix in contentlayer.

P.S.
Contentlayer is not currently being actively maintained and I am not sure if my pull request will be accepted, so I am planning to change to mdx-bundler or @next/mdx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants