remark-embedder / core

🔗 Remark plugin to convert URLs to embed code in markdown.
MIT License
118 stars 6 forks source link

Add Unified TS return types #4

Closed osiux closed 3 years ago

osiux commented 3 years ago

What: Seems like remark plugins written in TypeScript had a signature they commonly return. For example I'm trying to use this plugin with next-mdx-remote and have the following error:

image

Why: So this plugin can be used as other remark plugins without issues.

How: Checked the code of a few (1, 2) remark plugins written in TS and added the corresponding Attacher and Transformer (as UnifiedTransformer) return types. Tests are running fine after the change.

Checklist:

Added Transformer type as UnifiedTransformer since there is already a type using that name, and I see it being used on, for exampe, oembed transformers package. I think this should be fine, but let me know if you think this should be changed (maybe EmbedderTransformer?).

Also tried to avoid changing to arrow functions, but was getting several other TS errors and found easier to type it using them. If this doesn't work I can try again using normal functions.

codecov[bot] commented 3 years ago

Codecov Report

Merging #4 (e9290f9) into main (f962659) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main        #4   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           52        54    +2     
  Branches        13        13           
=========================================
+ Hits            52        54    +2     
Impacted Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)

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 f962659...e9290f9. Read the comment docs.

osiux commented 3 years ago

I think it should be typed ok, but still having some type issues when implemented on my site, not sure if it's a cache thing or something. Functionality wise it's working pretty good. Tried the immediate return as suggested by @MichaelDeBoey but doesn't work, has errors on build time.

I'm gonna take another look later to see if I can spot what's going on. If anyone wants to take a look, the error I'm having is: image

But if i add @ts-ignorelike here, the plugin does it's work, which means is just a type error somewhere. image

Maybe @wooorm has an idea on what I'm missing (?)

wooorm commented 3 years ago

I’m not a fan of TS personally so also have no real knowledge to see what’s going on.

But it might be that mdx-remote or mdx have incorrect types? 🤷‍♂️

osiux commented 3 years ago

Created a small project which uses remark directly, with the example in this repo, and don't get any type error related to this change, so I think @wooorm may be right on that type errors are from something else. I feel good merging this, but leave the decision to @kentcdodds or @MichaelDeBoey

kentcdodds commented 3 years ago

@All-contributors please add @osiux for code

allcontributors[bot] commented 3 years ago

@kentcdodds

I've put up a pull request to add @osiux! :tada:

kentcdodds commented 3 years ago

@All-contributors please add @MichaelDeBoey for review

allcontributors[bot] commented 3 years ago

@kentcdodds

I've put up a pull request to add @MichaelDeBoey! :tada:

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 1.2.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

kentcdodds commented 3 years ago

@All-contributors please add @wooorm for review

allcontributors[bot] commented 3 years ago

@kentcdodds

I've put up a pull request to add @wooorm! :tada: