4 ms·
This line is tested against and super optimized. By requiring this, you are future proof. The maintainer is active and writes tests. It's also a bit esoteric an
by Raphmedia 7y ago
This line is tested against and super optimized. By requiring this, you are future proof. The maintainer is active and writes tests. It's also a bit esoteric and hard to undestand.
`return process && (process.platform === 'win32' || /^(msys|cygwin)$/.test(process.env.OSTYPE));`
Here's is what's probably going on in the head of the people who require it:
"What does this do exactly? Why do I need to test against 'win32'? Should I also test against 'win64' or 'x64'? What if it changes later on? Do I need to keep track of Windows versions and then change all my apps? What if I write it without testing for 'cygwin' and someone else in the project do? Will the app only work partially on some hardware? What if some new OS comes out?"
Don't get me wrong, it's not perfect and is a security hole. But that's the reason it's done.
- darepublic 7y agoMy process for requiring it would be as follows: * need to figure out if platform is windows * stack overflow search (via google) unfruitful * npm search - popular lib for this! * it's got a lot of stars! * yarn add is-windows
- OskarS 7y agoYou really think that line is "super-optimized"? It generates and executes a regex to check if a string has one of two different prefixes. I don't know a lot about JavaScript internals, but I would imagine that is both forces memory to be allocated (and garbage generated) as well as being significantly slower than a basic prefix check. If doing this is considered "super-optimized", I don't even know what this industry is any more.
- ChrisSD 7y agoIt's not even testing prefixes, it's testing the whole string. It's the equivalent of: process.env.OSTYPE === 'cygwin' || process.env.OSTYPE === 'msys' Though I don't know enough about V8 to say how each is optimised.
- OskarS 7y agoOh, you're right! I missed the dollar sign! That makes it even worse! Unless V8 converts the regex to that expression (which it could maybe do, but I doubt), there's no way the regex version could be anything like as fast. Your version has the added benefit of also being far more readable and totally without any kind of memory allocation. Extraordinary package, this.
- Raphmedia 7y agoHere, test it. If you test it multiple time you get slightly different results. Both seem to be similar. You would have to ask the author why they went with the regex solution to know why they chose that one. Knowing Javascript, there's probably a pitfall we are not thinking about. http://jsben.ch/uFK8t http://jsben.ch/uFK8t
- Raphmedia 7y agoSuper optimized as in the sense that this line was distilled to be as small and lightweight as you can get it while making sure it works with actual tests. Just search for "Javascript check if variable is window" to find all kinds of solutions that seem to work at first glance but would probably end up breaking under some specific situations. Some people tests against the .toString of the variable, others look to see if the variable has under it what 'window' usually contains while some create a new instance of 'window' and compare it to their variable, etc. The solution that "is-window" brings seems to be as perfect as you are going to get it. In an ecosystem where things change quickly and you are not sure where your code is running it's easy to become dependent to such crutches. You do not want the mental burden of remembering all the hacks. I am not defending this particular package and I'm sure we could come with a better solution of testing it. If you do, make a pull request. I am merely explaining why those package happen.