TypeError: s.trim is not a function on rpm and deb target

[jviotti]

• Version: 19.9.1

• Target: deb and rpm

I'm suddenly getting the following issue while upgrading from 18.6.2 to 19.9.1:

TypeError: s.trim is not a function
    at isEmptyOrSpaces (/etcher/node_modules/electron-builder-util/src/util.ts:167:25)
    at checkNotEmpty (/etcher/node_modules/electron-builder/src/util/packageMetadata.ts:42:9)
    at checkMetadata (/etcher/node_modules/electron-builder/src/util/packageMetadata.ts:59:3)
    at /etcher/node_modules/electron-builder/src/packager.ts:150:5
    at next (native)
    at runCallback (timers.js:672:20)
    at tryOnImmediate (timers.js:645:5)
    at processImmediate [as _immediateCallback] (timers.js:617:5)
From previous event:
    at Packager.build (/etcher/node_modules/electron-builder/out/packager.js:219:11)
    at /etcher/node_modules/electron-builder/src/builder.ts:250:40
    at next (native)
From previous event:
    at build (/etcher/node_modules/electron-builder/out/builder.js:69:21)
    at Object.args [as handler] (/etcher/node_modules/electron-builder/src/cli/cli.ts:41:4)
    at Object.self.runCommand (/etcher/node_modules/electron-builder/node_modules/yargs/lib/command.js:233:22)
    at Object.Yargs.self._parseArgs (/etcher/node_modules/electron-builder/node_modules/yargs/yargs.js:1018:24)
    at Object.get [as argv] (/etcher/node_modules/electron-builder/node_modules/yargs/yargs.js:927:19)
    at Object.<anonymous> (/etcher/node_modules/electron-builder/src/cli/cli.ts:36:15)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:389:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:504:3

The stack trace leads me to this line:

checkNotEmpty("version", metadata.version)

Which means that somehow metadata.version is not a string, only on the deb and rpm targets (all other targets seem to work fine).

Check https://travis-ci.org/resin-io/etcher/jobs/249658677

Here's the electron-builder.yml configuration file: https://github.com/resin-io/etcher/blob/541ee6505b768750eb2cb2ece58ae3261281a5e8/electron-builder.yml

[jviotti]

Oh, I think I know what's going on! We manually override the versions of rpm and deb packages by using extraMetadata. For example:

--extraMetadata.version=$(APPLICATION_VERSION_REDHAT)

@develar recently changed extraMetadata to cast values to numbers or booleans, so my version is probably being casted to something that's not a string, and thus the error.

[jviotti]

BTW, we do some minor transformations to version strings to make them rpm or deb compatible:

APPLICATION_VERSION_DEBIAN = $(shell echo $(APPLICATION_VERSION) | tr "-" "~")
APPLICATION_VERSION_REDHAT = $(shell echo $(APPLICATION_VERSION) | tr "-" "~")

I wonder if electron-builder should support this out of the box.

In this case, we replace - (for example from 1.0.0-beta.19) to ~.

[jviotti]

Hm, even though coerceTypes looks like this:

export function coerceTypes(host: any): any {
  for (const key of Object.getOwnPropertyNames(host)) {
    const value = host[key]
    if (value === "true") {
      host[key] = true
    }
    else if (value === "false") {
      host[key] = false
    }
    else if (value === "null") {
      host[key] = null
    }
    else if (value != null && typeof value === "object") {
      coerceTypes(value)
    }
  }
  return host
}

It looks like a version string should remain a string.

[jviotti]

How odd! This is how metadata.version looks like:

version: [ '1.0.0+541ee65', '1.0.0+541ee65' ],

[jviotti]

The value seems to come that way from the config.extraMetadata property.

[jviotti]

This is what DEBUG=electron-builder reveals:

  electron-builder extraMetadata:
  electron-builder   version:
  electron-builder     - 1.0.0+541ee65
  electron-builder     - 1.0.0+541ee65
  electron-builder   name: etcher-electron
  electron-builder   analytics:
  electron-builder     updates:
  electron-builder       enabled: false
  electron-builder  +0ms

[jviotti]

Oh, I get it. If the metadata already contains a property x, and to set x again using --config.extraMetadata.x, then the value is appended, and not replaced!

[develar]

coerceTypes on Yargs level.

[lurch]

the value is appended, and not replaced!

@jviotti Does that also mean that for the builds where we turn off updates, we get enabled: [true, false] ?

EDIT: Ah, nope. Looks like the problem here is because we're actually setting --extraMetadata.version=foo twice in the same command line (here and here), which could be considered a bug in our Makefile, rather than a bug in electron-builder ;-)

[jviotti]

@lurch is right. I think there is a confusion about the root cause of this issue.

We have a package.json that contains a version:

{
    "version": "1.0.0"
}

If we then also pass --extraMetadata.version=2.0.0 when calling build, then the final package.json inside the asar will look like this:

{
    "version": [ "1.0.0", "2.0.0" ]
}

So basically, if we try to override a value from package.json using extraMetadata, then the value is not replaced, but both values remain, creating a list.

[jviotti]

This used to work before (I'll try to bisect if I have time), which is why I think a bug report is appropriate.

[lurch]

@jviotti Please re-read my message carefully ;-)
I believe (although I haven't tested) that the error isn't because we're defining a value with --extraMetadata that already exists in the package.json - surely if that was the case then we'd also get errors when we turn off updates?

Instead, I think that the error is that we're using --extraMetadata to set the same metadata twice from the same command-line, and it's this that is causing the values to get converted into an array. (and since there's no - in $(APPLICATION_VERSION), we have $(APPLICATION_VERSION_DEBIAN) being the
same as $(APPLICATION_VERSION))

Like I said I haven't actually tested this though, so please accept my apologies if I'm wrong!

[develar]

@jviotti we have tests for that case and it works. As @lurch said — problem is that you pass it several times and as result Yargs convert value to array.

[jviotti]

@develar @lurch Oh, I see. I'm really sorry for the whole confusion, you guys are right. I didn't notice we were also setting the version in the Makefile, so ignore me! Thanks a lot for the patience :)

[lurch]

I'll let you off this time @jviotti 😆 😆 😉