defunctzombie / node-process

process information for node.js and browsers
MIT License
122 stars 62 forks source link

process.platform #55

Open calvinmetcalf opened 8 years ago

calvinmetcalf commented 8 years ago

we should probably set this to 'browser', thoughts ?

defunctzombie commented 8 years ago

Seems simple enough.

defunctzombie commented 8 years ago

Tho then someone will complain saying it should be the user agent :p

calvinmetcalf commented 8 years ago

well they'll probably really want it to be firefox or chrome and totally be to young to remember the history of user agents

On Sat, Mar 19, 2016 at 2:29 AM Roman Shtylman notifications@github.com wrote:

Tho then someone will complain saying it should be the user agent :p

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/defunctzombie/node-process/issues/55#issuecomment-198647899

dead-claudia commented 7 years ago

@defunctzombie @calvinmetcalf

I feel process.platform should just be "browser". It's a single API (complete with entirely different path handling and file management), and testing for user agent is hardly useful at best, especially when you're just doing basic environment detection. Asking for process.platform to reflect the user agent is like asking it to be "debian", "fedora", etc. in Linux distros instead of just "linux" in Node.js itself.

CMCDragonkai commented 6 years ago

I vote for browser as well. For now I have to check for process.browser === true instead of just checking process.platform.

zenflow commented 6 years ago

@CMCDragonkai if (process.browser === true) { is not any more code than if (process.platform === 'browser') { ... ? Not sure what case process.platform is simpler in... ?

Also the === true bit is redundant (i.e. use just if (process.browser) {) unless you need the result to always be an actual Boolean and not just truthy-falsy.

CMCDragonkai commented 6 years ago

It just means I can use a switch case expression on process.platform instead of checking against a browser property and the platform property.

calvinmetcalf commented 6 years ago

pull requests are accepted wink wink

vmx commented 6 years ago

I've made a module to make the process.platform resemble the Node.js ones. This is useful for testing when you test on Node.js as well as on Browsers and need to skip tests based on the platform: https://github.com/ipfs/browser-process-platform/

dead-claudia commented 6 years ago

@vmx I would consider it unusual to need that. Browsers are sufficiently uniform across operating systems (we only need to remap key bindings, not paths, etc.) that for most use cases, it's pointless to draw such a distinction. Also, when it comes to skipping tests like that, I typically prefer controlling how I invoke the runner rather than in the tests themselves.

vmx commented 6 years ago

@isiahmeadows It's indeed uncommon. In my case some HTTP API is tested which is not fully implemented on all platforms. Hence I skip tests based on the platform the Browser is running on.

CMCDragonkai commented 6 years ago

@defunctzombie We have a PR for the platform field. https://github.com/defunctzombie/node-process/pull/82