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

Use ESM #71

Merged
merged 1 commit into from
Apr 26, 2021
Merged

Use ESM #71

merged 1 commit into from
Apr 26, 2021

Conversation

wooorm
Copy link
Member

@wooorm wooorm commented Apr 25, 2021

Initial checklist

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

Description of changes

  • Refactor code style
  • Update dependencies, dev-dependencies

Note: I’ll update mdast-util-to-string later, which causes a small change in output. And I’ll update to use JSDoc based types in another PR too — turned off dtslint meanwhile

* Refactor code style
* Update dependencies, dev-dependencies
@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Apr 25, 2021
Copy link
Member

@BarryThePenguin BarryThePenguin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its interesting to see how code can change overtime.. especially with the introduction of esm. Nice!

@wooorm
Copy link
Member Author

wooorm commented Apr 25, 2021

@BarryThePenguin any particular parts or things that changed you found interesting? Agreed, but curious to hear your thoughts!

Comment on lines -16 to +19
console.log('javascript', require('util').inspect(table, {depth: 3}))
console.log('javascript', inspect(table, {depth: 3}))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably more specific to remark-usage being used but..

I've never really been a fan of using require inline like this. ESM forces static imports to be defined at the beginning of the module.

Sure, there are dynamic imports. However, now that code path needs to be asynchronous

Comment on lines -39 to +38
var siblings = parent.children
var tail = siblings[siblings.length - 1]
var index = -1
var item
const siblings = parent.children
const tail = siblings[siblings.length - 1]
let index = -1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not ESM related.. but the distinction of which var became a let or const is interesting. Seeing the distinction like this is useful 👍🏻

Also, the scope of item changing from function scope to block scope 👀

@wooorm wooorm merged commit 4fdcf45 into main Apr 26, 2021
@wooorm wooorm deleted the esm branch April 26, 2021 07:44
@github-actions github-actions bot added 🤷 no/invalid This cannot be acted upon 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤷 no/invalid This cannot be acted upon 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging this pull request may close these issues.

None yet

3 participants