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

Add browser support via WebPack, Browserify and similar tools #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LinusU
Copy link

@LinusU LinusU commented Oct 23, 2017

Hi @3rd-Eden,

This patch moves all the logic for the parsing from index.js to lib/parse.js. The only thing left in index.js is the updater part. It then adds a browser.js which exports lib/parse.js directly. Finally a browser field is specified in the package.json to let WebPack and friends know which file to use.

These are the errors that I'm getting with the currently published version:

ERROR in ./node_modules/tmp/lib/tmp.js
Module not found: Error: Can't resolve 'fs' in './node_modules/tmp/lib'
 @ ./node_modules/tmp/lib/tmp.js 12:11-24
 @ ./node_modules/useragent/lib/update.js
 @ ./node_modules/useragent/index.js
 @ ./app.js

ERROR in ./node_modules/useragent/lib/update.js
Module not found: Error: Can't resolve 'fs' in './node_modules/useragent/lib'
 @ ./node_modules/useragent/lib/update.js 7:9-22
 @ ./node_modules/useragent/index.js
 @ ./app.js

ERROR in ./node_modules/useragent/lib/update.js
Module not found: Error: Can't resolve 'request' in './node_modules/useragent/lib'
 @ ./node_modules/useragent/lib/update.js 14:14-32
 @ ./node_modules/useragent/index.js
 @ ./app.js

ERROR in ./node_modules/useragent/lib/update.js
Module not found: Error: Can't resolve 'yamlparser' in './node_modules/useragent/lib'
 @ ./node_modules/useragent/lib/update.js 15:11-32
 @ ./node_modules/useragent/index.js
 @ ./app.js

I think that this is a much better approach than for me to install yamlparser and request since they will then be present in my final bundle.

Personally, I would very much prefer to remove the update functionality altogether. I don't think that 1) the API for consuming it are that good (there is no way to know when the new sources are used), 2) it's good practice to modify the javascript files inside node_modules at runtime and 3) not to know exactly what's being deployed since the files are changing when starting up... Anyhow, just my 2¢...

I also see that there are a lot of open issues and pull requests here. If you are low on time or just want a hand I would love to become a maintainer :)

@LinusU
Copy link
Author

LinusU commented Mar 15, 2018

Hey @3rd-Eden, have had time to give this any thought? I'd be happy to rebase on latest master if you think this is a good approach 👍

@LinusU
Copy link
Author

LinusU commented May 7, 2018

ping @3rd-Eden

@LinusU
Copy link
Author

LinusU commented Jun 18, 2018

ping @3rd-Eden ☺️

@LinusU
Copy link
Author

LinusU commented Aug 14, 2018

ping @3rd-Eden ☺️

@3rd-Eden
Copy link
Owner

Apologies for the late response. I don't know if it makes sense to make the library working for browsers because the way that the detection is done is not really suitable for web. It requires you to download a regular expression file that is a whooping 200kb.

@LinusU
Copy link
Author

LinusU commented Aug 20, 2018

Hmm, that is quite large yes 🤔 I wonder how well it compresses...

@jsardev
Copy link

jsardev commented Jan 2, 2019

@3rd-Eden Using webpack or any other bundlers doesn't necessarily mean it is being used in client-side. I'm trying to use this library in a universal app using next.js which bundles the server and client-side code with webpack which makes impossible to use this library. Would be great to resolve this issue.

@ngfelixl
Copy link

Same here. Would be great to use this library with Angular universal server side rendering, which requires webpack server support.

@pasiba
Copy link

pasiba commented Mar 1, 2019

@3rd-Eden my use case is a CloudFlare worker where I unfortunately cannot use NPM and so this PR might be useful for cases like that.

@3rd-Eden
Copy link
Owner

@Rusya44 Right, that seems like a valid usecase.

@nicollecastrog
Copy link

Dunno if this is helpful, but I'm using next.js (which in turn uses Webpack), and taking inspiration from this comment on next.js#7755, in my next.config.js I did:

module.exports = {
  webpack: (config, { isServer }) => {
    // Fixes server-side only packages used by 'useragent'
    if (!isServer) {
      config.node = {
        fs: "empty",
        net: "empty",
        tls: "empty"
      };
    }
    return config;
  }
}

... I also had to yarn add yamlparser, and then it all worked!

@johnelliott
Copy link

@3rd-Eden my use case is a CloudFlare worker where I unfortunately cannot use NPM and so this PR might be useful for cases like that.

Also wanting to use this in Cloudflare workers.

@pasiba
Copy link

pasiba commented May 15, 2020

@johnelliott Currently I am using this solution https://github.com/browserslist/browserslist-useragent-regexp

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 this pull request may close these issues.

None yet

7 participants