unifiedjs / unified

☔️ interface for parsing, inspecting, transforming, and serializing content through syntax trees

https://unifiedjs.com

MIT License

4.49k stars 110 forks source link

Update typings to support ESM #141

Closed ChristianMurphy closed 3 years ago

ChristianMurphy commented 3 years ago

Initial checklist

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

Description of changes

Update typings to support ESM style imports (part of https://github.com/unifiedjs/unified/commit/dc46bc5a85d038566e226c388b58ff937341cbcf) For now keeping the typings as a .d.ts file, JSDoc could be nice down the road, but overload support and split interfaces (FrozenProcessor vs Processor) are still challenging in JSDoc and potentially more difficult to maintain than the .d.ts file.

codecov-commenter commented 3 years ago

Codecov Report

Merging #141 (53d7245) into main (4c927c8) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #141   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          452       452           
=========================================
  Hits           452       452

Impacted Files	Coverage Δ
index.js	`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 4c927c8...53d7245. Read the comment docs.

ChristianMurphy commented 3 years ago

:notebook: the removal of namespace makes this diff look larger than it is. Disable space in the preview for a better view at changes (https://github.blog/2021-04-28-github-desktop-hiding-whitespace-expanding-diffs-repo-aliases/#hide-whitespace-in-diffs)

ChristianMurphy commented 3 years ago

/cc @remcohaszing

ChristianMurphy commented 3 years ago

It could, updated in 699b97a

wooorm commented 3 years ago

Want to bring https://github.com/rehypejs/rehype-remark/pull/6#discussion_r660266691 in directly here, or shall we do it separately?

ChristianMurphy commented 3 years ago

Want to bring https://github.com/rehypejs/rehype-remark/pull/6#discussion_r660266691 in directly here, or shall we do it separately?

Sure, I added some test cases for it 12b8820. It looks like I may have misspoke in https://github.com/rehypejs/rehype-remark/pull/6#discussion_r660904145 it appears that this is already supported, though it's good to have test cases to show/confirm it.

wooorm commented 3 years ago

@ChristianMurphy I think you checked the wrong part. This is the part that seems wrong: https://github.com/unifiedjs/unified/blob/4c927c86d3c4a484101e6ac7ece0321d0f281862/types/index.d.ts#L317-L321

ChristianMurphy commented 3 years ago

Updated in 53d7245

github-actions[bot] commented 3 years ago

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.

ChristianMurphy commented 3 years ago

This will be a part of unified version 10 release