Modernize & cleanup #72

gmaclennan · 2016-11-10T20:35:35Z

This is based upon the original source code and api from v1.2.5. This is the same code that is published in the latest v1.3.8.

I don't know webpack, but this should work fine webpack if you use that, would be great if someone could test it.

I have kept the API from v1.2.5 and code logic is largely the same, the changes are mainly to restructure the code as commonJS modules, and update code formatting (to match the latest .eslintrc in the project (with changes to work with the latest eslint v3.9.1). The code changes are:

client fingerprints (browser and server) are cached rather than calculated each time, since they do not change.
Should work in WebWorkers and ServiceWorkers now - it uses self if window is undefined.
Very simple fix for react-native which apparently does not have userAgent or mimeTypes defined. This id does not probably change much between clients, so we should have something with better entropy.

I threw away many of the changes in master, which used ES2016 and required a complicated build process with transforms etc. The project should now be a lot simpler.

Browser builds rely on the browser field in package.json which is supported by both browserify and webpack.

Standalone UMD builds are created in dist which is excluded from the git repo but included in npm, so that they will be available from https://unpkg.com/cuid/dist/cuid.js https://unpkg.com/cuid/dist/cuid.min.js once this package is published to npm. The build script runs as a prepublish script, so it should require nothing more than npm publish.

Tests are running and passing locally and on saucelabs. Hopefully travis-ci should just work.

Finally, sorry about the lack of commit messages, but the repo seemed to have drifted a lot from the last published version and there was no logical step-by-step development from the current master.

gmaclennan · 2016-11-10T20:44:10Z

Looks like saucelabs credentials need updated in .travis.yml.

ericelliott · 2016-11-10T22:40:01Z

CONTRIBUTING.md

-export SAUCE_USERNAME=username
-export SAUCE_ACCESS_KEY=key
+sauce_username: my_awesome_username
+sauce_key: 550e8400-e29b-41d4-a716-446655440000


This isn't a real key, is it?

If the key comes from .zuulrc, which is not included in the repo, and not encrypted - how do we run the sauce labs tests on Travis?

This isn't a real key, is it?

Nope. Just a random number. I should make that more clear. Copied and pasted from the Zuul wiki.

If the key comes from .zuulrc, which is not included in the repo, and not encrypted - how do we run the sauce labs tests on Travis?

.zuulrc is for contributors who want to run the tests in their own saucelabs account.

For running tests from travis you should store encrypted environment variables following these instructions: https://github.com/defunctzombie/zuul/wiki/Travis-ci

I had assumed the current encrypted env variables in the .travis.yml were for Saucelabs, but seems like they are no longer valid. If you like I could try and set up a saucelabs account for this, unless you already have one. Saucelabs requires a separate account for each opensource project for their free tier.

I have a Sauce Labs account for this, but as I mentioned in the notes below, it doesn't work for forked PRs.

ericelliott · 2016-11-10T22:46:15Z

lib/fingerprint.browser.js

@@ -0,0 +1,12 @@
+var pad = require('./pad.js');
+
+var env = typeof window === 'object' ? window : self;


What is self?

self is a reference to the global scope (equivalent to window) in WebWorkers and ServiceWorkers: https://developer.mozilla.org/en-US/docs/Web/API/WorkerGlobalScope/self

I looked at a couple of modules designed to work in both a browser context and in WebWorkers and this was the pattern they use.

Interesting. TIL.

Can you add tests for Web Workers and Service Workers?

what about node?

node > self ReferenceError: self is not defined > window ReferenceError: window is not defined > global ...

What about this point? Was this resolved? I get a

var env = typeof window === 'object' ? window : self; ^ ReferenceError: self is not defined

in node; any suggestions??

ericelliott · 2016-11-10T23:19:39Z

Notes:

https://docs.travis-ci.com/user/pull-requests#Pull-Requests-and-Security-Restrictions
https://docs.travis-ci.com/user/encryption-keys/
Travis has some dedicated, secure config for sauce connect. Any way to make this work with the zuul config?

gmaclennan · 2016-11-11T00:54:34Z

https://docs.travis-ci.com/user/pull-requests#Pull-Requests-and-Security-Restrictions

Just reading through this. It's likely that the encrypted variables in the current .travis.yml are valid saucelabs credentials, it's just that they are not accessible to travis from my PR because it is from a different repo. Hopefully when this is merged to master (or another branch) these tests should just run with those existing credentials.

ericelliott · 2016-11-15T03:13:06Z

Any chance we can get a zuul test for the Web Worker/Service Worker fix?

gmaclennan · 2016-11-15T04:19:43Z

I'm not sure this test actually works. I'm not sure whether the TAP output from the worker thread gets back to zuul so that it can confirm the tests are run. This does run the test in a worker, I'm just not sure about how to handle output. I've never run tests in a worker before.

gmaclennan · 2016-11-15T04:27:14Z

A little further info:

When I run zuul locally and run zuul --local 8080 --ui tape -- test/test-worker.js it seems like the test passes, but if I run them both together with a glob it seems to fail: zuul --local 8080 --ui tape -- test/**/*.js

ericelliott · 2017-03-14T00:08:05Z

What do we need to do to get these checks passing so we can close all those open issues?

gmaclennan · 2017-03-14T17:47:50Z

Thanks for following up on this.

The tests are currently failing because of failing saucelab credentials, because this is a PR from a different repo. The only way to fix this it to merge this PR into this repo so that the tests can run with the correct credentials.

I suggest merging this PR into a new branch here, checking the tests pass, then merging into master.

The one thing missing is that I don't think the tests for this working in a webworker are actually working - i.e. if the tests fail in a webworker I don't think this currently cause the test output to show failure. However, this PR has better test coverage than the current version even without this.

nealoke · 2017-04-18T11:07:03Z

@ericelliott any update on this? :)

ericelliott · 2017-04-19T05:37:07Z

@nealoke @gmaclennan I've invited both of you as collaborators. Feel free to do what you need to do in order to get the tests passing on the main repo.

nealoke · 2017-04-20T08:43:49Z

@ericelliott thanks but i'm not really familiar with the CI tools :( When I have time i'll dive into this!

MarkHerhold · 2017-09-01T01:06:49Z

I put a few hours into trying to get this to work with SauceLabs and have not been able to get consistent results. Would it suffice to run the Karma tests on Chrome Headless (v60) and forget the other browsers? Or perhaps try a different testing service?

These are the errors I am seeing:

31 08 2017 21:00:10.117:WARN [IE 11.0.0 (Windows 8.1 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.
IE 11.0.0 (Windows 8.1 0.0.0) ERROR
  Disconnected, because no message in 10000 ms.

ericelliott · 2017-09-01T02:18:53Z

@MarkHerhold I believe you can adjust the timeout. I'd prefer to make sure it works in IE11, because it currently still holds about 13% market share...

ericelliott · 2017-09-01T02:20:24Z

I think I'd be OK with running the tests in Chrome headless for now just to get this back on track. We could release it as a breaking change, just to be on the safe side. However, I'd want somebody to test it in IE11 and at least spot check it for us... Do you happen to have IE11 available?

MarkHerhold · 2017-09-01T03:21:34Z

@ericelliott I've increased the timeout to 120 seconds (see this commit on my fork) and set retries to two and still have flaky results.

Everyone can download an IE11 VM (and other versions) here.

I checked IE11 manually and the build does indeed work (tested cuid.js).

I will work on getting Chrome Headless working on Travis CI.

ericelliott · 2017-09-01T04:08:21Z

Looking great. Thanks!

Modernize & cleanup (#72)

ericelliott · 2018-01-04T06:49:21Z

Published as v2.0.1

Modernize

277eb11

gmaclennan mentioned this pull request Nov 10, 2016

Fix for webworkers #70

Closed

Update CONTRIBUTING.md

a09c06f

ericelliott requested changes Nov 10, 2016

View reviewed changes

Clarify that saucelabs key needs replacing.

b7b06ed

add worker test

c0c4ed1

ericelliott approved these changes Apr 19, 2017

View reviewed changes

ericelliott mentioned this pull request Aug 14, 2017

navigator is not defined #40

Closed

ericelliott mentioned this pull request Aug 29, 2017

Status of cuid #82

Closed

MarkHerhold mentioned this pull request Sep 1, 2017

Modernize & cleanup (#72) #83

Merged

ericelliott merged commit c0c4ed1 into paralleldrive:master Sep 6, 2017

ericelliott added a commit that referenced this pull request Sep 6, 2017

Merge pull request #83 from MarkHerhold/modernize

5f11022

Modernize & cleanup (#72)

Flet mentioned this pull request Jan 3, 2018

Remove all references to Applitude #41

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize & cleanup #72

Modernize & cleanup #72

gmaclennan commented Nov 10, 2016

gmaclennan commented Nov 10, 2016

ericelliott Nov 10, 2016

ericelliott Nov 10, 2016

gmaclennan Nov 11, 2016

ericelliott Nov 11, 2016

ericelliott Nov 10, 2016

gmaclennan Nov 11, 2016

ericelliott Nov 11, 2016

zargold May 8, 2018 •

edited

aateeque Sep 11, 2019

ericelliott commented Nov 10, 2016 •

edited

gmaclennan commented Nov 11, 2016

ericelliott commented Nov 15, 2016

gmaclennan commented Nov 15, 2016

gmaclennan commented Nov 15, 2016

ericelliott commented Mar 14, 2017

gmaclennan commented Mar 14, 2017

nealoke commented Apr 18, 2017

ericelliott commented Apr 19, 2017

nealoke commented Apr 20, 2017

MarkHerhold commented Sep 1, 2017

ericelliott commented Sep 1, 2017

ericelliott commented Sep 1, 2017

MarkHerhold commented Sep 1, 2017 •

edited

ericelliott commented Sep 1, 2017

ericelliott commented Jan 4, 2018

		@@ -0,0 +1,12 @@
		var pad = require('./pad.js');

		var env = typeof window === 'object' ? window : self;

Modernize & cleanup #72

Modernize & cleanup #72

Conversation

gmaclennan commented Nov 10, 2016

gmaclennan commented Nov 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zargold May 8, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericelliott commented Nov 10, 2016 • edited

gmaclennan commented Nov 11, 2016

ericelliott commented Nov 15, 2016

gmaclennan commented Nov 15, 2016

gmaclennan commented Nov 15, 2016

ericelliott commented Mar 14, 2017

gmaclennan commented Mar 14, 2017

nealoke commented Apr 18, 2017

ericelliott commented Apr 19, 2017

nealoke commented Apr 20, 2017

MarkHerhold commented Sep 1, 2017

ericelliott commented Sep 1, 2017

ericelliott commented Sep 1, 2017

MarkHerhold commented Sep 1, 2017 • edited

ericelliott commented Sep 1, 2017

ericelliott commented Jan 4, 2018

zargold May 8, 2018 •

edited

ericelliott commented Nov 10, 2016 •

edited

MarkHerhold commented Sep 1, 2017 •

edited