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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not construct dirname on Linux #20

Merged
merged 1 commit into from
May 24, 2022
Merged

Do not construct dirname on Linux #20

merged 1 commit into from
May 24, 2022

Conversation

timdp
Copy link
Contributor

@timdp timdp commented May 17, 2022

I'm bundling term-size using esbuild, which handles import.meta.url a bit differently than other bundlers.

Now, in my case, the fileURLToPath call is dead code, because I'm running my bundle on Linux, where term-size doesn't need to invoke any commands. So I was thinking it could be more lazy, and coincidentally fix the issue I'm facing.

This PR moves the __dirname emulation into a helper function that's only called on macOS and Windows.

I've also put the line-splitting in there because I prefer DRY code, but it's a bit esoteric this way. Would you prefer an explicit splitLines (or even a dependency on split-lines)? Or alternatively, I could make a less invasive PR and only extract the __dirname construction into a helper function, for example.

... Or we could find a better way to deal with the lack of __dirname in ESM. 馃槈

@sindresorhus
Copy link
Owner

I usually don't like to change my perfectly valid code to please buggy/incomplete bundlers, but since this is a small change, I'll accept it.

@sindresorhus sindresorhus merged commit fd2daa1 into sindresorhus:main May 24, 2022
@timdp
Copy link
Contributor Author

timdp commented May 24, 2022

I feel your pain. Thanks!

@timdp timdp deleted the dirname-optional branch May 24, 2022 10:51
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.

None yet

2 participants