Node-slack-sdk: pjson reads incorrect package.json?

Created on 9 Mar 2018  ·  2Comments  ·  Source: slackapi/node-slack-sdk

First of all big props to everyone involved in shipping v4. Very excited to use this 😍 ✨ 🍾

Description

I'm in the process of moving our code over to use v4 of the node-slack-sdk

The first error our tests were failing on was this one.

It looks like in the instantiation you set the user agent, based on some values in the package.json of the library consumer. In our case we have neither a name nor a version defined in our package.json, so all of our test were failing with TypeError: Cannot read property 'replace' of undefined.

After setting values for name and version, we got more tests to pass

I assume the user agent should be instantiated based on the package name and version of @slack/client as opposed to the ultimate consumer of this library? Or at least in my experience that's how other http request libraries tend set the user agent 😬

https://github.com/slackapi/node-slack-sdk/blob/df853215b86a777c01a5299a2a508313f0ffd0c9/src/util.ts#L20

What type of issue is this? (place an x in one of the [ ])

  • [X] bug
  • [ ] enhancement (feature request)
  • [ ] question
  • [ ] documentation related
  • [ ] testing related
  • [ ] discussion

Requirements (place an x in each of the [ ])

  • [X] I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • [X] I've read and agree to the Code of Conduct.
  • [X] I've searched for any related issues and avoided creating a duplicate issue.

Reproducible in:

@slack/client version: 4.0.0

node version: v8.9.3

Steps to reproduce:

  1. Have a package.json without name or version
  2. Instantiate the web client
  3. Profit
bug

All 2 comments

You're right, it is supposed to work by getting the name and version of @slack/client. I'll have to take a look into how pjson is not doing what I expected it to do. This is definitely related to #466.

the solution i'm proposing involves directly requiring package.json (same as #466). i'm open to other ideas. otherwise i anticipate having a fix out early next week.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dcsan picture dcsan  ·  12Comments

thepont picture thepont  ·  17Comments

aoberoi picture aoberoi  ·  10Comments

amkoehler picture amkoehler  ·  13Comments

bobrik picture bobrik  ·  25Comments