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

Export issue #17

Closed
lehaBay opened this issue Dec 21, 2015 · 11 comments
Closed

Export issue #17

lehaBay opened this issue Dec 21, 2015 · 11 comments

Comments

@lehaBay
Copy link

lehaBay commented Dec 21, 2015

Hi!

problem is in this line: https://github.com/jackspirou/clientjs/blob/master/src/client.js#L683
in minified version before this line exports = q is executed, q is a UAParser here which is a function.

so typeof exports === 'object' is false here

so "module.exports = ClientJS;" is never executed so when one require clientjs he has a UAParser instead.

hope that makes sense

@jackspirou
Copy link
Owner

Hi @lehaBay thanks for creating this issue.

You mean this line in the minified version correct? https://github.com/jackspirou/clientjs/blob/master/dist/client.min.js#L21

@jackspirou
Copy link
Owner

Ah yes... I found it in the original UAParser source:

if(typeof module!==UNDEF_TYPE&&module.exports){
    exports=module.exports=UAParser
}
exports.UAParser=UAParser

I wonder if its best to simply remove those lines or remove typeof exports === 'object' in the ClientJS source.

Thoughts?

@lehaBay
Copy link
Author

lehaBay commented Dec 22, 2015

I think typeof exports === 'object' should be replaced with typeof exports !== "undefined"

but actually there is one more problem.
there is a line from minified version:
if ("undefined" !== typeof exports)"undefined" !== typeof module && module.exports && (exports = module.exports = q), exports.UAParser = q; else {
f.UAParser = q;

so, window.UAParser isn't set if 'exports' is defined so this line throws an error:
e = (new UAParser).getResult();

@lehaBay
Copy link
Author

lehaBay commented Dec 22, 2015

@jackspirou

so it works if replace
e = (new UAParser).getResult();
with
e = (new (window.UAParser||exports.UAParser)).getResult();

and replace typeof exports === 'object' in the ClientJS with typeof exports !== 'undefined'

and one more thing, looks like library installed using npm is older than one in repository. It doesn't have getCustomFingerprint method

@jackspirou
Copy link
Owner

@lehaBay I ended up replacing the minified versions with the regular source code in the vendors directories since those get minified anyway by the build.sh bash script.

After reviewing the comments from the regular source it looks like the code that was causing the issue is optional so I commented it out. You can find it here: https://github.com/jackspirou/clientjs/blob/master/src/vendor/ua-parser.js#L866

As for exports === 'object' i took your suggestion and replaced it with exports !== "undefined".

Also I updated the npm registry with the latest version numbers that also include these changes.

Let me know what you think and if we still need to make some changes or if we can close this issue. Thanks!

@lehaBay
Copy link
Author

lehaBay commented Jan 3, 2016

@jackspirou i'm having this error now "Uncaught ReferenceError: UAParser is not defined", what i mentioned in the comment above resolves this and everything else is fine now

@jackspirou
Copy link
Owner

@lehaBay the code you have suggested in the comment above (without markup) is in reference to the minified version. Im glad you got the minified version to work, but really we should be updating the source file, namely src/client.js and the vendor file src/vendor/ua-parser.js which is why I referenced it above.

@lehaBay
Copy link
Author

lehaBay commented Jan 3, 2016

@jackspirou ok, i thought it's easy to find this line in the sources. here it is:
https://github.com/jackspirou/clientjs/blob/master/src/client.js#L139

just replace var parser = new UAParser(); with var parser = new (window.UAParser||exports.UAParser);

@jackspirou
Copy link
Owner

@lehaBay it wasn't obvious to me 😜

Thanks for sticking through this issue and helping me get this to work with requirejs. I don't use this project with requirejs myself, so its great to have someone give feedback.

I made the updates and now we are on npm clientjs version 0.1.8. Let me know if we are good with requrejs now :)

@lehaBay
Copy link
Author

lehaBay commented Jan 4, 2016

@jackspirou Now it's fine, thank you for fixing it, i found another issue with getCustomFingerprint but i'll create a new one because it's not related to this one

@jackspirou
Copy link
Owner

@lehaBay Awesome thanks again for your help!

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

No branches or pull requests

2 participants