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

Support Node, Browsers, WebWorkers, & ReactNative #68

Closed
4 tasks done
ericelliott opened this issue Oct 15, 2016 · 4 comments
Closed
4 tasks done

Support Node, Browsers, WebWorkers, & ReactNative #68

ericelliott opened this issue Oct 15, 2016 · 4 comments

Comments

@ericelliott
Copy link
Collaborator

ericelliott commented Oct 15, 2016

The JS version of cuid needs to support all of these targets:

We'll use feature detection to select the correct entropy source and return the right fingerprint() function for the environment.

Ideally, we should select entropy sources supported by all of the above environments.

@gmaclennan
Copy link
Contributor

Do you know what kind of problem keeping different versions has been? The adhoc browser field spec in package.json I think is supported by both browserify and webpack. I think for browsers the issue with feature detection is extra package size, and you would need a special build anyway to ignore node specific modules (e.g. the node version does require('os'). I think the build scripts and code organization can be simplified however, and can take a look at this when I have spare moments.

@ericelliott ericelliott changed the title Unify cuid builds Support Node, Browsers, WebWorkers, & ReactNative Nov 9, 2016
@ericelliott
Copy link
Collaborator Author

Fair point. I'm not actually picky about whether or not there are separate builds. Just that we support everything that needs supporting. Ideally, we can find entropy sources that work everywhere, but that shouldn't be a strict requirement.

@gmaclennan
Copy link
Contributor

Sorry to keep questioning: seeing if I can help clean this up / modernize in the best way. Re. linting / code style, the older source files used a different code style and lint settings that the new files. I see from #31 that you prefer custom eslint over standard. I should format the code to match the current .eslintrc in master right?

I see the newer code is written in ES2016 and is transformed with babel. I see no need for this for a module like this, it just increases the barrier for contributors, adds an additional build step, and increases code size. Any preferences here? Happy to stay in ES2016 too if preferred.

Finally what browser versions should this target? I see the latest .zuul.yml targets IE >= 9, which would be great. IE 8 compatibility could add some overhead, although could be left for the user to do with polyfills. (e.g., using Object.keys(window) as a much simpler technique in browser-fingerprint)

@ericelliott
Copy link
Collaborator Author

I see the newer code is written in ES2016 and is transformed with babel. I see no need for this for a module like this, it just increases the barrier for contributors, adds an additional build step, and increases code size. Any preferences here? Happy to stay in ES2016 too if preferred.

This module is small/simple enough that ES5 + node-style modules is fine.

IE >= 9 + all major current browsers (including mobile Safari) is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants