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

Fix #4645, Add an extension to _mocha for people with loaders #4647

Closed
wants to merge 4 commits into from

Conversation

FossPrime
Copy link

@FossPrime FossPrime commented Jun 3, 2021

Description of the Change

Alternate Designs

  • Moving mocha to mocha.js: Docs hard reference mocha without extension.
  • Making a copy of _mocha to mocha.cjs: This is uncommon. Though it's strongly recommended by Node and others when you transpile to CJS from a native ES Module. https://redfin.engineering/node-modules-at-war-why-commonjs-and-es-modules-cant-get-along-9617135eeca1
  • Loading mocha with a PassThrough stream in mocha.js: That a little exotic and depends on stdio
  • We could make a mjs wrapper, but that seems unnecessary right now... it is a workaround however
// mocha-4645.mjs
import cli from './node_modules/mocha/lib/cli/index.js'
cli.main()

Why should this be in core?

It cleanly allows for TS and ESM support, using loaders. A recurring long standing request.

Benefits

ts-node/esm and other node loaders will work as expected. As well as supporting custom loaders, custom loaders for logging, performance, etc.

Possible Drawbacks

If something resolves the symlinks in .bin/ manually AND looks for a /mocha$/ regex... this would break their code. I don't know any reason to do that and they would using multiple levels of non public APIs, so I'm dismissing this.

Applicable issues

#4645

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.344% when pulling 62d8708 on rayfoss:node-loader-bugfix into a93d759 on mochajs:master.

@FossPrime FossPrime closed this Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants