Skip to content

errors: replace manual sep code with path.sep #32440

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

errors: replace manual sep code with path.sep #32440

wants to merge 1 commit into from

Conversation

MylesBorins
Copy link
Contributor

@nodejs-github-bot nodejs-github-bot added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Mar 23, 2020
@MylesBorins MylesBorins mentioned this pull request Mar 23, 2020
Closed
4 tasks
@ljharb
Copy link
Member

ljharb commented Mar 23, 2020

LGTM, thanks!

@MylesBorins
Copy link
Contributor Author

There is a circular reference here causing a problem

TypeError: Cannot destructure property 'ERR_INVALID_ARG_TYPE' of 'require(...).codes' as it is undefined.

@MylesBorins
Copy link
Contributor Author

I think we might want to consider closing this. As path is reliant on errors.js there is a circular reference that break all the things. Even if we broke out the windows / unix separator into a different file we would still need to manually do the process check due to the way path is architected (manually creates both POSIX + Windows and then decides export based on process.platform).

So I think with that being said the simplest thing to do is nothing 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evanlucas evanlucas evanlucas approved these changes

@ZYSzys ZYSzys ZYSzys approved these changes

Assignees
No one assigned
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MylesBorins @ljharb @evanlucas @ZYSzys @nodejs-github-bot