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

Only transform file import path if not already transformed #4901

Closed

Conversation

christian-bromann
Copy link
Contributor

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions.

Description of the Change

Passing in a file path, e.g. file:///foo/bar in Node causes problems as Mocha tries to resolve the path, e.g.:

path.resolve('file:///foo/bar.spec.js') // returns "/path/to/cwd/file:/foo/bar.spec.js"

Alternate Designs

n/a

Why should this be in core?

This seems to be a bug in core.

Benefits

It allows Mocha to use file urls. This is important for Windows where import('d:\foo\bar.spec.js') is not possible.

Possible Drawbacks

n/a

Applicable issues

n/a

@coveralls
Copy link

coveralls commented Jul 15, 2022

Coverage Status

Coverage increased (+0.002%) to 94.329% when pulling 0d9992a on christian-bromann:cb-respect-file-urls into 023f548 on mochajs:master.

@erwinheitzman
Copy link

@christian-bromann let's get the linting fixed (spaces vs tabs issue?) and hopefully the team will look at this :)

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2023

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 6, 2023
@github-actions github-actions bot removed the stale this has been inactive for a while... label Jan 9, 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 10, 2023
@christian-bromann
Copy link
Contributor Author

Closing as it seems the project is not interested in the change.

@christian-bromann christian-bromann deleted the cb-respect-file-urls branch May 10, 2023 18:55
@JoshuaKGoldberg
Copy link
Member

👋 coming back to this @christian-bromann, 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!

Note that I don't see any linked issues. The contributing guidelines right now ask for that so that folks can discuss the change first. We'll continue to require that after #5038. Was there any impetus for this PR besides what's in the description and linked webdriverio issues?

@christian-bromann
Copy link
Contributor Author

Hey @JoshuaKGoldberg 👋 thanks so much for reaching out. Unfortunately I am not exactly sure what the underlying issue was. I think I struggled with path issues in Windows when porting the WebdriverIO code base to ESM. I am not sure how I worked around it but there isn't any issue anymore, for me at least.

Thanks for picking up the maintenance on this 🙏

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.

4 participants