syntax-tree / mdast-util-to-hast

utility to transform mdast to hast
https://unifiedjs.com
MIT License
101 stars 42 forks source link

all, one, etc should be re-exported in main index.js? #53

Closed ignatiusreza closed 3 years ago

ignatiusreza commented 3 years ago

Initial checklist

Subject

The exported toHast support passing options.handlers to provide customized logic to process certain node. The documentation for this options suggest to "Take a look at lib/handlers/ for examples.".

Most of the handlers under lib/handlers/ are written using the helper function all, one, etc. So it seems quite reasonable for custom handlers to also want to use those helper functions.

Problem

Pre-ESM migration, we can import/require directly from mdast-util-to-hast/lib/all and it will worked.

Post-ESM migration, .mjs file can't import from mdast-util-to-hast/lib/all.js since it is written using esm syntax, but does not have .mjs extension (seems like for direct file import, the type: module config in package.json is ignored?)

Solution

in index.js, we should reexport the helper functions

export {all} from './lib/all.js'
export {one} from './lib/one.js'
export {toHast} from './lib/index.js'

so that we can import it like import { all, toHash } from 'mdast-util-to-hast';..

Alternatives

Copy+paste the helper functions?

wooorm commented 3 years ago

Post-ESM migration, .mjs file can't import from mdast-util-to-hast/lib/all.js since it is written using esm syntax, but does not have .mjs extension (seems like for direct file import, the type: module config in package.json is ignored?)

It should work. It sounds like your tooling or project isn’t set up to support ESM?

github-actions[bot] commented 3 years ago

Hi! This was marked as ready to be worked on! Note that while this is ready to be worked on, nothing is said about priority: it may take a while for this to be solved.

Is this something you can and want to work on?

Team: please use the area/* (to describe the scope of the change), platform/* (if this is related to a specific one), and semver/* and type/* labels to annotate this. If this is first-timers friendly, add good first issue and if this could use help, add help wanted.

wooorm commented 3 years ago

That being said, exporting them from index.js is probably a good idea! Is that something you can work on? (adding some docs for them would be needed too)

ignatiusreza commented 3 years ago

It should work. It sounds like your tooling or project isn’t set up to support ESM?

oh interesting, it does work with webpack, but I had the issue when using jest, so I assumed that the same issue would happened in vanilla nodejs.. but, testing it out, it does work in vanilla nodejs, so seems like it is jest that need to have better support..

Is that something you can work on?

I opened PR #54

github-actions[bot] commented 3 years ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.