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

docs: document Node.js' --enable-source-maps flag #4173

Merged
merged 6 commits into from
Jun 17, 2020

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Feb 1, 2020

Description of the Change

Newer versions of Node.js introduce the --enable-source-maps flag which, when enabled, caches source-maps and provides accurate stack traces to users running transpiled code, e.g., TypeScript.

Benefits

Many users of Node.js are writing TypeScript, this allows them to receive better stack traces in mocha without installing any dependencies.

Drawbacks

There's still work to be done to polish this feature in Node.js (would very much appreciate getting feedback).

@jsf-clabot
Copy link

jsf-clabot commented Feb 1, 2020

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Feb 1, 2020

Coverage Status

Coverage increased (+0.03%) to 93.467% when pulling edb8e6b on bcoe:note-about-source-maps into cb5eb8e on mochajs:master.

@juergba juergba added the area: documentation anything involving docs or mochajs.org label Feb 2, 2020
@juergba
Copy link
Contributor

juergba commented Feb 2, 2020

@bcoe thank you for this PR.

We do not whitelist or document NodeJs options in our documentation. There are just a few exceptions like --inspect/--inspect-brk or --experimental-modules.
IMO we should not start doing this, it's a bottomless task.
In our repo mocha-examples we offer some working samples incl. TS setups, maybe that's the place to recommend the --enable-source-maps flag.

@bcoe
Copy link
Contributor Author

bcoe commented Feb 2, 2020

IMO we should not start doing this, it's a bottomless task.

I understand 100% where you're coming from.

Worth considering though, something like 60% of folks writing JavaScript these days spend at least some time writing TypeScript. Rather than just documenting this flag, it might be worth having a section of docs that speaks to making mocha work well with TypeScript:

  1. speaking to which reporters work well with TypeScript (I believe some have source-map-support as a dependency?).
  2. documenting the -enable-source-maps flag.
  3. speaking to the source-map-support library.

@boneskull
Copy link
Contributor

I think this is worth adding. I realize that documenting node flags is a slippery slope (as @juergba wrote, there's no end to it), but I think we should take it on a case-by-case basis. It's plainly useful information, and I'd like to see more of this on the docs site. That said, we should add to the examples repo.

@bcoe can you please resolve the conflict?

Copy link
Contributor

@boneskull boneskull left a comment

Choose a reason for hiding this comment

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

please address comments + resolve conflicts

docs/index.md Outdated Show resolved Hide resolved
@boneskull boneskull added the semver-patch implementation requires increase of "patch" version number; "bug fixes" label Mar 12, 2020
@bcoe
Copy link
Contributor Author

bcoe commented Mar 19, 2020

@boneskull 👍 will try to get this done ASAP.

@bcoe bcoe force-pushed the note-about-source-maps branch 2 times, most recently from b042a0e to 4739f0f Compare March 24, 2020 04:54
Copy link
Contributor

@outsideris outsideris left a comment

Choose a reason for hiding this comment

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

It is under COMMAND-LINE USAGE now.
How about we document it under ABOUT node FLAGS because it is a node flag not mocha flag.

@boneskull
Copy link
Contributor

@bcoe The build is failing because there's an incorrect anchor link. Also, @outsideris had a request

@bcoe bcoe force-pushed the note-about-source-maps branch from 1315101 to edb8e6b Compare May 21, 2020 03:30
@bcoe
Copy link
Contributor Author

bcoe commented May 21, 2020

@boneskull done 👍

@bcoe bcoe requested a review from boneskull May 21, 2020 03:55
Copy link
Contributor

@outsideris outsideris left a comment

Choose a reason for hiding this comment

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

Thank you.

@juergba juergba merged commit 80863b1 into mochajs:master Jun 17, 2020
@juergba juergba added this to the next milestone Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: documentation anything involving docs or mochajs.org semver-patch implementation requires increase of "patch" version number; "bug fixes"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants