-
Notifications
You must be signed in to change notification settings - Fork 633
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
refactor(path): make isWindows
check compatible with Node and Bun
#4961
Merged
iuioiua
merged 18 commits into
denoland:main
from
BenMcLean981:is-windows-cross-runtime
Aug 29, 2024
Merged
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
e782830
Refactored to get rid of unnecessary function.
BenMcLean981 ce50f95
Changed browser version of isWindows.
BenMcLean981 0146d96
Refactored getIsWindows.
BenMcLean981 962bf8b
Added node/bun version of isWindows.
BenMcLean981 548372f
Got cross platform os module working.
BenMcLean981 5d37157
Handled Deno error.
BenMcLean981 fa7d78a
Fixed type errors.
BenMcLean981 19a295f
Merge branch 'main' into is-windows-cross-runtime
BenMcLean981 056e5a5
Removed node:os import.
BenMcLean981 6e66ec4
Merge branch 'is-windows-cross-runtime' of https://github.com/BenMcLe…
BenMcLean981 90486a5
Merge branch 'main' into is-windows-cross-runtime
BenMcLean981 681761e
Merge branch 'main' into is-windows-cross-runtime
iuioiua bac530b
tweaks
iuioiua 54acb71
fix
iuioiua 87caadc
cleanup
iuioiua 6f9ee95
tweak
iuioiua 47cccd6
fix
iuioiua aabaca9
Added support for node.
BenMcLean981 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,78 @@ | ||
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. | ||
// This module is browser compatible. | ||
|
||
// Keep this up-to-date with Deno.build.os | ||
export const isWindows: boolean = getIsWindows() === true; | ||
|
||
function getIsWindows(): boolean | undefined { | ||
return ( | ||
getIsWindowsOnDeno() || getIsWindowsOnBrowser() || getIsWindowsOnNodeOrBun() | ||
); | ||
} | ||
|
||
/** | ||
* Operating system type, equivalent to the type of | ||
* {@linkcode https://deno.land/api?s=Deno.build | Deno.build.os}. | ||
* @returns whether the os is windows or undefined if not running | ||
* in a deno runtime, undefined if not a deno runtime. | ||
*/ | ||
type OSType = | ||
| "darwin" | ||
| "linux" | ||
| "windows" | ||
| "freebsd" | ||
| "netbsd" | ||
| "aix" | ||
| "solaris" | ||
| "illumos"; | ||
|
||
const osType: OSType = (() => { | ||
function getIsWindowsOnDeno(): boolean | undefined { | ||
// deno-lint-ignore no-explicit-any | ||
const { Deno } = globalThis as any; | ||
if (typeof Deno?.build?.os === "string") { | ||
return Deno.build.os; | ||
return Deno.build.os === "windows"; | ||
} | ||
} | ||
|
||
/** | ||
* @returns whether the os is windows or undefined if not running | ||
* in a web browser, undefined if not a web browser | ||
*/ | ||
function getIsWindowsOnBrowser(): boolean | undefined { | ||
// deno-lint-ignore no-explicit-any | ||
const { navigator } = globalThis as any; | ||
if (navigator?.appVersion?.includes?.("Win")) { | ||
return "windows"; | ||
|
||
return containsWindows(navigator?.userAgent); | ||
} | ||
|
||
/** | ||
* according to documentation node's os module is implemented | ||
* in bun as well. | ||
* {@link https://bun.sh/docs/runtime/nodejs-apis#node-os} | ||
* | ||
* @returns whether the os is windows or undefined if not running | ||
* on node or bun runtime | ||
*/ | ||
function getIsWindowsOnNodeOrBun() { | ||
const os = tryGettingNodeOsModule(); | ||
|
||
return containsWindows(os?.version()); | ||
} | ||
|
||
type OsModule = { | ||
version(): string; | ||
}; | ||
|
||
// deno-lint-ignore no-explicit-any | ||
declare const require: any; | ||
|
||
function tryGettingNodeOsModule(): OsModule | undefined { | ||
try { | ||
return getNodeOsModule(); | ||
} catch { | ||
return undefined; | ||
} | ||
} | ||
|
||
return "linux"; | ||
})(); | ||
function getNodeOsModule(): OsModule | undefined { | ||
if (require !== undefined) { | ||
return require("os"); | ||
} else { | ||
return undefined; | ||
} | ||
} | ||
|
||
export const isWindows = osType === "windows"; | ||
function containsWindows(s?: string): boolean | undefined { | ||
if (typeof s === "string") { | ||
return s.toUpperCase().includes("WINDOWS"); | ||
} else { | ||
return undefined; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If node.js is in ESM mode (.mjs or
type: module
in package.json), then it doesn't haverequire
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I tested by manually converting to js and then running with
node os.js
, explains why this worked for me then.I'll mark this PR as Draft again until I can come up with a better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
@std
is published in JSR, and it's distributed as ESM in Node.js. So I think it's enough to only support ESM context.