NodeJS reality check

gleemonk

You've heard it before: The Nodejs ecosystem is a shitfest of bad code. It has been my assumption that this is an overparticularization of the rule that code is shit wherever you decide to look. This assumption has slowly been eroded over the last years as I've been exposed to more and more Node packages. Let me excerpt.

Given that the parallelshell package is widely used, you'd expect that it was written by people with a clue. Representative line:

cwd: process.versions.node < '8.0.0' ? process.cwd : process.cwd(),

Apparently Node changed it's API from process.cwd being a property to it being a method. That's fair, Nodejs 8 was a major release, so why not pull a stunt like this? You're only going to piss off everybody using your API.

Now, what happened to parallelshell once node 10 was released? Well, quickfix happened:

cwd: parseInt(process.versions.node) < 8 ? process.cwd : process.cwd(),

This is robust. It even works if process.versions were suddenly written in hexadecimal.

dkf

@gleemonk

ben_lubar

I should make a npm module named call-if-function-or-just-return-the-goddamn-string-if-its-already-a-string.

Bulb

@gleemonk said in NodeJS reality check:

cwd: process.versions.node < '8.0.0' ? process.cwd : process.cwd(),

Hm, I'd personally write it as:

cwd: typeof(process.cwd) === 'function' ? process.cwd() : process.cwd,

because why test an unrelated condition when I can test a related one.

Now of course it wouldn't be JavaScript if even a test for being a function wasn't complicated. The accepted answer here suggests:

{}.toString.call(f) === '[object Function]'

and while the most upvoted one does have the simple typeof f === 'function', I actually kinda like this one:

!!(f && f.constructor && f.call && f.apply);

because in a duck-typed language, everything is a function if it quacks like one, i.e. has call and apply methods. It is also fastest, because it is not comparing any strings.

Zerosquare

@ben_lubar : please make it.

And then, a few years later, remove it. So we can relive the whole leftpad debacle and have a good laugh.

blakeyrat

@gleemonk If they want to know if it's a function or not, why not just ask JS if typeof blah.cwd == 'function'? Then you could get rid of this fragile version number bullshit.

gordonjcp

@blakeyrat Because that's not enterprisey enough.

sockpuppet7

@Bulb What is the point of using a dynamic language, then manually checking the types of everything?

Magus

@sockpuppet7 Because you can't trust that your code works otherwise. Congratulations, you've discovered why dynamic typing is bad.

loopback0

@gleemonk said in NodeJS reality check:

Given that the parallelshell package is widely used, you'd expect that it was written by people with a clue.

Given that it's widely used in the Node ecosystem, I'd expect otherwise.

blakeyrat

@sockpuppet7 said in NodeJS reality check:

@Bulb What is the point of using a dynamic language, then manually checking the types of everything?

Smart people use TypeScript and just transpile into JS once all the type guarantees are in place.

Gribnit

@blakeyrat Although some smart people may do this, I think the way the statement implies that this is a smart thing to do is misleading.

swayde

@blakeyrat said in NodeJS reality check:

@sockpuppet7 said in NodeJS reality check:

@Bulb What is the point of using a dynamic language, then manually checking the types of everything?

Smart people use TypeScript and just transpile into JS once all the type guarantees are in place.

The best thing about typescript is breaking changes in minor versions. Makes me super happy.

pie_flavor

@sockpuppet7 said in NodeJS reality check:

@Bulb What is the point of using a dynamic language, then manually checking the types of everything?

Ask @ben_lubar

PJH

@gleemonk said in NodeJS reality check:

cwd: process.versions.node < '8.0.0' ? process.cwd : process.cwd(),

...

cwd: parseInt(process.versions.node) < 8 ? process.cwd : process.cwd(),

Version comparison is hard.

Nor is it solely constrained to Node.

The Old New Thing

Version numbers. Very important. And so many people check them wrong.
This is why Windows 95's GetVersion function returned 3.95 instead of 4.0. A lot of code checked the version number like this:
  UINT Ver = GetVersion();
  UINT MajorVersion = LOBYTE(uVer);
  UINT MinorVersion = HIBYTE(uVer);
  if (MajorVersion < 3 || MinorVersion < 10) {
   Error("This program requires Windows 3.1");
  }

ben_lubar

@PJH said in NodeJS reality check:

@gleemonk said in NodeJS reality check:
cwd: process.versions.node < '8.0.0' ? process.cwd : process.cwd(),
...
cwd: parseInt(process.versions.node) < 8 ? process.cwd : process.cwd(),
Version comparison is hard.

Nor is it solely constrained to Node.

The Old New Thing
Version numbers. Very important. And so many people check them wrong.
This is why Windows 95's GetVersion function returned 3.95 instead of 4.0. A lot of code checked the version number like this:
  UINT Ver = GetVersion();
  UINT MajorVersion = LOBYTE(uVer);
  UINT MinorVersion = HIBYTE(uVer);
  if (MajorVersion < 3 || MinorVersion < 10) {
   Error("This program requires Windows 3.1");
  }

See also: Java thinking Windows 9 is too old.

LB_

@PJH C++ devs have version checks so easy these days...

#include <array>

int main()
{
    return std::array{4, 0} < std::array{3, 10};
}

Before I knew this trick I used to implement the logic manually and it always felt wrong. When I have to use other languages I prefer to find an existing solution...

Gribnit

@LB_ You bastards. You sheer cheeky operator overloading bastards.

Scala should be able to do this, wonder if it do.

PleegWat

@LB_ What about 5.3rc6?

dcon

@PJH said in NodeJS reality check:

Version comparison is hard.

Nor is it solely constrained to Node.

The Old New Thing
Version numbers. Very important. And so many people check them wrong.
This is why Windows 95's GetVersion function returned 3.95 instead of 4.0. A lot of code checked the version number like this:
  UINT Ver = GetVersion();
  UINT MajorVersion = LOBYTE(uVer);
  UINT MinorVersion = HIBYTE(uVer);
  if (MajorVersion < 3 || MinorVersion < 10) {
   Error("This program requires Windows 3.1");
  }

I say, let'm burn.

pie_flavor

@Gribnit said in NodeJS reality check:

@LB_ You bastards. You sheer cheeky operator overloading bastards.

Scala should be able to do this, wonder if it do.

Rust is even better - you can do this with tuples.

ben_lubar

@LB_ said in NodeJS reality check:

@PJH C++ devs have version checks so easy these days...
#include <array>

int main()
{
    return std::array{4, 0} < std::array{3, 10};
}
Before I knew this trick I used to implement the logic manually and it always felt wrong. When I have to use other languages I prefer to find an existing solution...

invalid operation: [2]int literal < [2]int literal (operator < not defined on array)

aww

sockpuppet7

@pie_flavor said in NodeJS reality check:

@Gribnit said in NodeJS reality check:

@LB_ You bastards. You sheer cheeky operator overloading bastards.

Scala should be able to do this, wonder if it do.

Rust is even better - you can do this with tuples.

and Android just solved it using a singe integer to be the api version internally

jnz

@ben_lubar
Trying to do std::array{4, 0} < std::array{3, 10} uses a C++17 feature called "Class template argument deduction". It will only work in a C++17 compiler. The code compiles with GCC and clang but both ICC and MSVC reject it even in C++17 mode. Maybe this is not guaranteed to work with std::array? I can see how it could deduce the type of the array from the arguments but deducing the number of elements in the array seems like it would be trickier. Maybe GCC and clang are supporting this as an extension or as an inadvertent side-effect of the way their STL is implemented.

We could help the compiler out by specifying the template arguments explicitly: std::array<int, 2>{4, 0} < std::array<int, 2>{3, 10}. That works on any compiler supporting C++11 but it is a bit wordy and it doesn't seem to generate very good code.

I would try using tuples instead: std::make_tuple(4, 0) < std::make_tuple(3, 10). This should work on any C++11 compiler and seems to generate good code in some limited tests. It is optimized down to return 0.

Interestingly I couldn't reproduce your error message in any compiler that I tried. Either a compiler will accept the code or get confused about the lack of template arguments and fail to parse it. No compiler that I tried would successfully parse this and then go on to complain that you're not allowed to compare arrays. I tried searching Google for that error message but that didn't produce anything enlightening. What compiler did it come from?

jmp

@jnz said in NodeJS reality check:

@ben_lubar
Trying to do std::array{4, 0} < std::array{3, 10} uses a C++17 feature called "Class template argument deduction". It will only work in a C++17 compiler. The code compiles with GCC and clang but both ICC and MSVC reject it even in C++17 mode. Maybe this is not guaranteed to work with std::array? I can see how it could deduce the type of the array from the arguments but deducing the number of elements in the array seems like it would be trickier. Maybe GCC and clang are supporting this as an extension or as an inadvertent side-effect of the way their STL is implemented.

We could help the compiler out by specifying the template arguments explicitly: std::array<int, 2>{4, 0} < std::array<int, 2>{3, 10}. That works on any compiler supporting C++11 but it is a bit wordy and it doesn't seem to generate very good code.

I would try using tuples instead: std::make_tuple(4, 0) < std::make_tuple(3, 10). This should work on any C++11 compiler and seems to generate good code in some limited tests. It is optimized down to return 0.

Interestingly I couldn't reproduce your error message in any compiler that I tried. Either a compiler will accept the code or get confused about the lack of template arguments and fail to parse it. No compiler that I tried would successfully parse this and then go on to complain that you're not allowed to compare arrays. I tried searching Google for that error message but that didn't produce anything enlightening. What compiler did it come from?

Probably whatever the Go compiler is called.

ben_lubar

@jmp said in NodeJS reality check:

Go compiler

gc