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: support canonical module #4888

Closed
wants to merge 2 commits into from

Conversation

JacobLey
Copy link
Contributor

Description of the Change

Handle canonicalizing Module, prevent implicit stringification failures

Alternate Designs

  • Have canonicalType return 'object' for modules (not chosen as that could be breaking, not sure...)
  • Have 'module' fallthrough to 'object' with no custom logic (not chosen so we can indicate different between "object" and "module")

Why should this be in core?

Current module canonicalization is broken, see #4887

Benefits

Tests can properly handle errors with a Module inside

Possible Drawbacks

Small chance there is some internal/custom handling expecting canonicalization of modules to fail... Seems unlikely?

Applicable issues

Fixes #4887

Bug fix, patch release

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 22, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.005%) to 94.332% when pulling 0264c7f on JacobLey:canonicalModule into 023f548 on mochajs:master.

@github-actions
Copy link
Contributor

This PR hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this PR will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label Sep 23, 2022
@JacobLey
Copy link
Contributor Author

Still an active issue

@github-actions github-actions bot removed the stale this has been inactive for a while... label Sep 26, 2022
@github-actions
Copy link
Contributor

This PR hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this PR will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label Jan 25, 2023
@JacobLey
Copy link
Contributor Author

Bump

@github-actions github-actions bot removed the stale this has been inactive for a while... label Jan 27, 2023
@github-actions
Copy link
Contributor

This PR hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this PR will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label May 29, 2023
@github-actions github-actions bot closed this Jun 12, 2023
@JoshuaKGoldberg
Copy link
Member

👋 coming back to this @JacobLey, are you still interested in working on this PR? As of #5027 there are maintainers who can review it now. No worries if you no longer have time - but if you do that'd be great!

@JacobLey
Copy link
Contributor Author

JacobLey commented Dec 4, 2023

Sure, from what I can tell the commit is easily merged with latest mainline, and the solution should still be the same.

I have updated the branch, but don't believe I have power to re-open this PR.

Opened a new PR with the same commits (rebased off master): #5040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this has been inactive for a while...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug: Modules in errors cause unhandledRejection, tests quit early
3 participants