db-migrate / pg

A postgresql driver for db-migrate.
Other
67 stars 51 forks source link

fix(addForeignKey): pass joined strings to `util.format` #42

Closed mmalecki closed 5 years ago

mmalecki commented 5 years ago

node v12 seems to no longer format arrays passed as strings by .toString()ing them, which effectively resulted in a .join(','), but returning in something to the tune of:

> util.format('%s', ['foo','bar'])
"[ 'foo', 'bar' ]"

While v11.15.0 returns:

> util.format('%s', ['foo','bar'])
'foo,bar'

This fixes this issue on v12, and doesn't break any other version.

Shall we add v10, v11 and v12 to .travis.yml?

wzrdtales commented 5 years ago

Thanks for your contribution! :tada:

Can you please elaborate on this? I just tested on node 12, 10 and 8 and all the behavior of an array in to string was the same. So it did not changed. However, using join is the proper way to do it anyways, so I will accept this PR, but I still look for clarification, since I do not see the change you're describing.

Thank you!

mmalecki commented 5 years ago

Hm, interesting! I wrote this helper:

VERSIONS=("6.17.1" "8.16.0" "10.15.3" "11.15.0" "12.1.0")
for version in "${VERSIONS[@]}"; do
  echo -n "$version:"
  docker run -i "node:$version" node -p 'util.format("%s", ["a", "b"])'
done

And its results are as follows:

6.17.1: a,b
8.16.0: a,b
10.15.3: a,b
11.15.0: a,b
12.1.0: [ 'a', 'b' ]

So it definitely seems like something changed, at least in 12.1.0, for our usecase of util.format. If I were to guess, it's related to the work on formatting iterables, but I hadn't had the time to read into the core source.

wzrdtales commented 5 years ago

Ah ok, b/c you wrote toString() I was misled. util.format then seems to have changed its behavior.

mmalecki commented 5 years ago

Sorry for confusion and thanks for merging!