Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc snippets did not help me to migrate to v2 #300

Closed
nicgirault opened this issue Mar 5, 2019 · 0 comments · Fixed by #301
Closed

doc snippets did not help me to migrate to v2 #300

nicgirault opened this issue Mar 5, 2019 · 0 comments · Fixed by #301

Comments

@nicgirault
Copy link
Contributor

Thanks for maintaining this project.

I found the code snippet misleading in the README:

  1. I was wondering what is Impression https://github.com/lancedikson/bowser/blob/master/README.md#browser-props-detection. Why do you need it? I would suggest to remove it

  2. Since it's written:

const browser = Bowser.getParser(window.navigator.userAgent);
impression.userTechData = browser.parse();
console.log(impression.userTechData);

// outputs
{
  browser: {
    name: "Internet Explorer"
    version: "11.0"
  },
  os: {
    name: "Windows"
    version: "NT 6.3"
    versionName: "8.1"
  },
  platform: {
    type: "desktop"
  },
  engine: {
    name: "Trident"
    version: "7.0"
  }
}

I was expecting to be able to write browser.parse().os but actually not because typescript says the parse method returns void. To output what is written as the output, I had to write: console.log(Bowser.parse(window.navigator.userAgent)).

I think the doc seriously need to be rewrote. If I can share my personal experience, I saw the v2 a couple of weeks ago and tried to migrate and I gave up because of this. Now that I'm taking the time to upgrade my dependencies, I took the time to dig into your code to upgrade but I was also looking for alternatives because I was thinking "it became to complicated..." whereas it is not.

The changelog and release note could also be clearer to know what users have to change to migrate from v1 to v2.

nicgirault added a commit to nicgirault/bowser that referenced this issue Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant