Feature request: TypeScript definition

[JunichiSugiura]

Do you have any plans to migrate to TypeScript?

Whether converting source code to TS or writing definition file, I can help on that so let me know 😉

[jescalan]

Yes I do, vaguely. I'm be open to either as a contribution if you're interested!

[JunichiSugiura]

Sure, I'm happy to contribute. Let me know how you wanna go about it and I'll start from there.

[jescalan]

I think the plan would be either to convert the source to TS or write definition files. Whatever you prefer, I am open to either one.

[JunichiSugiura]

I'll work on this on Saturday.

[JunichiSugiura]

Hmmm, seems like mdx-js is working on TypeScript support on v2.

We'll have better Options type once v2 is merged and upgrade the dependency accordingly in this repo. I may leave the type as {[key: string]: any} for now.

[Howard86]

Hey guys, first of all, thank you all for this package!
I am using this in my own blog, and quickly forked & rewrote one in TS.

However, I have found some issues after the rework, so I consider simply writing types for this module instead.
(I haven't really check @JunichiSugiura 's version from npm yet, so results can vary)

If you are interested, I have published my forked version on npm for testing purposes. This package will be removed within 3 days following npm policy.

TL;DR

• My own type declaration if interested
• My own npm in TS (will be unpublished soon)

Static page build size is huge after my rework

I am not too sure about the reason (if someone can help I will be more than appreciated), but this puts me off and sticks to the original npm package.

TS version

├ ● /blog/[slug]                                               2.02 MB        2.11 MB
├   ├ /blog/2020-10-07
├   └ /blog/2020-10-21

Original with types

├ ● /blog/[slug]                                               2.86 kB        85.9 kB
├   ├ /blog/2020-10-07
├   └ /blog/2020-10-21

After verifying the network in both pages (build && start), next has built the package in the page.

Caveats

Somehow as using fs module in TS version, a next.config.js is required with the following to pass build and test

module.exports = {
  webpack: (config, { isServer }) => {
    // Fixes npm packages that depend on `fs` module
    if (!isServer) {
      config.node = {
        fs: "empty",
      };
    }
    return config;
  },
};

Conclusion

Stick to type declaration for me, and possibly supporting mdx-js rework in TS.

I guess it also relates to the way I use this module outside of /page folder. Will test again after refactoring!

[jescalan]

@Howard86 wow, this is great - thanks for the thorough rundown!

I would guess honestly that this is your reason for the large bundle size, maybe? In theory this shouldn't be an issue, but I don't know how the code transpiles through, and that seems like the only candidate.

Regardless, I will probably take a serious look at rewriting this package in typescript at some point, but until then just having it ship with types I think would solve everyone's problems, and I'd be happy to accept a contribution to those ends.

[Howard86]

hmmm interesting...
I have found this in TS Handbook and been using this syntax in production. Maybe I should try using export type & export for separation of concerns

[aoberoi]

👋 If there's anything I can do to help move this issue forward, I'm happy to help.

An update: There are versions of @mdx-js/mdx published under the dist-tag next which already have TypeScript definitions. This should be useful in situations like @JunichiSugiura mentioned above about needing an Options type.

This might not be the right place to ask, but @jescalan, any thoughts or plans regarding support for v2 of @mdx-js/mdx? I haven't taken a close look at the breaking changes, but I'd expect this could necessitate a new major version of this package as well.

Would it be helpful for me to investigate the build size issue that @Howard86 ran into above? That port of this package is no longer available (as noted, deleted after 3 days). Would it be useful for me to contribute an experimental port as a PR to a new branch on this repo?

[jescalan]

There's a PR here that adds types - if you could test it on your project(s) and give it a review that would be very helpful @aoberoi - I'm planning to ship it today.

I am planning on supporting mdx version 2 as soon as i can after its release. It will be a major release to this package when it is supported.

If anyone feels capable of completing a functional rewrite of this package in typescript, I would welcome this. So far there have been attempts but nobody has been able to get it across the line. I think shipping native typings should solve the problem for most though.

[Ehesp]

@jescalan I started having a play around with the rewrite here: https://github.com/Ehesp/next-mdx-remote

Some points:

• Uses MDX v2
• Is built on top of Refactor hydrate and renderToString #39 (v2)
• Tried to add Rollup, but getting an error when converting the MDXRemote component into JS which I can't work out.

My knowledge of the whole MDX ecosystem isn't great, but hopefully this helps.

EDIT: Also worth adding, since MDX v2 exposes typings, there is a lot less faffing around with manual types.

[raulfdm]

I just tried v2 but it seems the types are not exported.

It means I cannot import the interfaces in my App and use them. 🤔

[jescalan]

@raulfdm Did you have a chance to check out the readme? https://github.com/hashicorp/next-mdx-remote#typescript

[raulfdm]

@raulfdm Did you have a chance to check out the readme? https://github.com/hashicorp/next-mdx-remote#typescript

Hey @jescalan. I hadn't, I'm sorry.

I had read the update from dependabot and thought it would have a default export via index.js, wrong assumption but thank you for the message.

[jescalan]

Closing as we now ship types!