From 4d52b90b0c099c05b795af13ab15f3b01005fcec Mon Sep 17 00:00:00 2001 From: Alexis Georges Date: Wed, 17 Oct 2018 13:38:00 -0700 Subject: [PATCH 01/20] Improve linting (#116) --- .eslintrc.yaml | 60 +++++++++++++++++++++++++++++-- src/EventProcessor.js | 2 +- src/EventSender.js | 6 ++-- src/GoalTracker.js | 2 +- src/Requestor.js | 2 +- src/Store.js | 6 ++-- src/Stream.js | 4 +-- src/__tests__/EventSource-mock.js | 2 +- src/__tests__/LDClient-test.js | 4 +-- src/__tests__/Store-test.js | 4 +-- src/index.js | 16 ++++----- src/messages.js | 2 ++ 12 files changed, 83 insertions(+), 27 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 2d50f0b6..c4b241e4 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -5,51 +5,105 @@ extends: env: es6: true node: true - browser: true plugins: - babel - prettier globals: VERSION: true + describe: true + it: true + expect: true + jest: true + beforeEach: true + afterEach: true + window: true + document: true rules: + # https://github.com/prettier/eslint-plugin-prettier prettier/prettier: - error + + # https://github.com/babel/eslint-plugin-babel + babel/semi: error + + # https://eslint.org/docs/rules/array-callback-return array-callback-return: error + + # https://eslint.org/docs/rules/curly curly: - error - all + + # https://eslint.org/docs/rules/no-implicit-coercion no-implicit-coercion: - 'off' - boolean: false number: true string: true allow: [] + + # https://eslint.org/docs/rules/no-eval no-eval: error + + # https://eslint.org/docs/rules/no-implied-eval no-implied-eval: error + + # https://eslint.org/docs/rules/no-param-reassign no-param-reassign: - error - props: true + + # https://eslint.org/docs/rules/no-return-assign no-return-assign: error + + # https://eslint.org/docs/rules/no-self-compare no-self-compare: error + + # https://eslint.org/docs/rules/radix radix: error + + # https://eslint.org/docs/rules/no-array-constructor no-array-constructor: error + + # https://eslint.org/docs/rules/no-new-wrappers no-new-wrappers: error + + # https://eslint.org/docs/rules/no-cond-assign no-cond-assign: error + + # https://eslint.org/docs/rules/no-use-before-define no-use-before-define: - error - functions: false + + # https://eslint.org/docs/rules/eqeqeq eqeqeq: error # Deprecations are required to turn enforce this camelcase: warn - + + # https://eslint.org/docs/rules/no-new-object no-new-object: error + + # https://eslint.org/docs/rules/no-nested-ternary no-nested-ternary: error + + # https://eslint.org/docs/rules/no-unused-vars no-unused-vars: error + + # https://eslint.org/docs/rules/no-var no-var: error + + # https://eslint.org/docs/rules/prefer-const prefer-const: error + + # https://eslint.org/docs/rules/prefer-arrow-callback prefer-arrow-callback: error + + # https://eslint.org/docs/rules/arrow-body-style arrow-body-style: - error - as-needed - babel/semi: error \ No newline at end of file + + # https://eslint.org/docs/rules/no-undef + no-undef: error diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 155a3205..6df2416d 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -31,7 +31,7 @@ export default function EventProcessor(eventsUrl, environmentId, options = {}, e samplingInterval = options.samplingInterval || 0; } - if (options.flushInterval !== undefined && (isNan(options.flushInterval) || options.flushInterval < 2000)) { + if (options.flushInterval !== undefined && (isNaN(options.flushInterval) || options.flushInterval < 2000)) { flushInterval = 2000; reportArgumentError('Invalid flush interval configured. Must be an integer >= 2000 (milliseconds).'); } else { diff --git a/src/EventSender.js b/src/EventSender.js index 8a4b7951..14149ae2 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -10,7 +10,7 @@ export default function EventSender(eventsUrl, environmentId, forceHasCors, imag const sender = {}; function loadUrlUsingImage(src, onDone) { - const img = new Image(); + const img = new window.Image(); if (onDone) { img.addEventListener('load', onDone); } @@ -34,7 +34,7 @@ export default function EventSender(eventsUrl, environmentId, forceHasCors, imag const jsonBody = JSON.stringify(events); const send = onDone => { function createRequest(canRetry) { - const xhr = new XMLHttpRequest(); + const xhr = new window.XMLHttpRequest(); xhr.open('POST', postUrl, !sync); utils.addLDHeaders(xhr); xhr.setRequestHeader('Content-Type', 'application/json'); @@ -76,7 +76,7 @@ export default function EventSender(eventsUrl, environmentId, forceHasCors, imag // Detect browser support for CORS (can be overridden by tests) if (hasCors === undefined) { if (forceHasCors === undefined) { - hasCors = 'withCredentials' in new XMLHttpRequest(); + hasCors = 'withCredentials' in new window.XMLHttpRequest(); } else { hasCors = forceHasCors; } diff --git a/src/GoalTracker.js b/src/GoalTracker.js index a384f663..7c949543 100644 --- a/src/GoalTracker.js +++ b/src/GoalTracker.js @@ -60,7 +60,7 @@ export default function GoalTracker(goals, onEvent) { const urls = goal.urls || []; for (let j = 0; j < urls.length; j++) { - if (doesUrlMatch(urls[j], location.href, location.search, location.hash)) { + if (doesUrlMatch(urls[j], window.location.href, window.location.search, window.location.hash)) { if (goal.kind === 'pageview') { onEvent('pageview', goal); } else { diff --git a/src/Requestor.js b/src/Requestor.js index 1ca30e6f..ced85ee9 100644 --- a/src/Requestor.js +++ b/src/Requestor.js @@ -5,7 +5,7 @@ import * as messages from './messages'; const json = 'application/json'; function fetchJSON(endpoint, body, callback, sendLDHeaders) { - const xhr = new XMLHttpRequest(); + const xhr = new window.XMLHttpRequest(); let data = undefined; xhr.addEventListener('load', () => { diff --git a/src/Store.js b/src/Store.js index 4a7cde19..f682d206 100644 --- a/src/Store.js +++ b/src/Store.js @@ -17,7 +17,7 @@ export default function Store(environment, hash, ident) { const key = getFlagsKey(); let dataStr, data; try { - dataStr = localStorage.getItem(key); + dataStr = window.localStorage.getItem(key); } catch (ex) { console.warn(messages.localStorageUnavailable()); return null; @@ -41,7 +41,7 @@ export default function Store(environment, hash, ident) { const key = getFlagsKey(); const data = utils.extend({}, flags, { $schema: 1 }); try { - localStorage.setItem(key, JSON.stringify(data)); + window.localStorage.setItem(key, JSON.stringify(data)); } catch (ex) { console.warn(messages.localStorageUnavailable()); } @@ -50,7 +50,7 @@ export default function Store(environment, hash, ident) { store.clearFlags = function() { const key = getFlagsKey(); try { - localStorage.removeItem(key); + window.localStorage.removeItem(key); } catch (ex) {} }; diff --git a/src/Stream.js b/src/Stream.js index d5527a7a..07548c1f 100644 --- a/src/Stream.js +++ b/src/Stream.js @@ -24,7 +24,7 @@ export default function Stream(baseUrl, environment, hash, config) { }; stream.isConnected = function() { - return es && (es.readyState === EventSource.OPEN || es.readyState === EventSource.CONNECTING); + return es && (es.readyState === window.EventSource.OPEN || es.readyState === window.EventSource.CONNECTING); }; function reconnect() { @@ -45,7 +45,7 @@ export default function Stream(baseUrl, environment, hash, config) { function openConnection() { let url; let query = ''; - if (typeof EventSource !== 'undefined') { + if (typeof window.EventSource !== 'undefined') { if (useReport) { // we don't yet have an EventSource implementation that supports REPORT, so // fall back to the old ping-based stream diff --git a/src/__tests__/EventSource-mock.js b/src/__tests__/EventSource-mock.js index 724cf11a..6e100933 100644 --- a/src/__tests__/EventSource-mock.js +++ b/src/__tests__/EventSource-mock.js @@ -44,7 +44,7 @@ export default function EventSource(url) { } } - function mockOpen() { + function mockOpen(error) { if (this.readyState === EventSource.CONNECTING) { this.readyState = EventSource.OPEN; this.onopen && this.onopen(error); diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index bcf02833..1688a160 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -239,7 +239,7 @@ describe('LDClient', () => { // sandbox.restore(window.localStorage.__proto__, 'getItem'); // sandbox.stub(window.localStorage.__proto__, 'getItem').throws(); - localStorage.getItem.mockImplementationOnce(() => { + window.localStorage.getItem.mockImplementationOnce(() => { throw new Error(); }); @@ -256,7 +256,7 @@ describe('LDClient', () => { }); it('should handle localStorage setItem throwing an exception', done => { - localStorage.setItem.mockImplementationOnce(() => { + window.localStorage.setItem.mockImplementationOnce(() => { throw new Error(); }); diff --git a/src/__tests__/Store-test.js b/src/__tests__/Store-test.js index 9da5efbf..649681f2 100644 --- a/src/__tests__/Store-test.js +++ b/src/__tests__/Store-test.js @@ -7,7 +7,7 @@ describe('Store', () => { it('should handle localStorage getItem throwing an exception', () => { const store = Store('env', 'hash', ident); - const getItemSpy = jest.spyOn(localStorage, 'getItem').mockImplementation(() => { + const getItemSpy = jest.spyOn(window.localStorage, 'getItem').mockImplementation(() => { throw new Error('localstorage getitem error'); }); @@ -22,7 +22,7 @@ describe('Store', () => { it('should handle localStorage setItem throwing an exception', () => { const store = Store('env', 'hash', ident); - const setItemSpy = jest.spyOn(localStorage, 'setItem').mockImplementation(() => { + const setItemSpy = jest.spyOn(window.localStorage, 'setItem').mockImplementation(() => { throw new Error('localstorage getitem error'); }); diff --git a/src/index.js b/src/index.js index 55130941..09a32950 100644 --- a/src/index.js +++ b/src/index.js @@ -237,10 +237,10 @@ export function initialize(env, user, options = {}) { function doNotTrack() { let flag; - if (navigator && navigator.doNotTrack !== undefined) { - flag = navigator.doNotTrack; // FF, Chrome - } else if (navigator && navigator.msDoNotTrack !== undefined) { - flag = navigator.msDoNotTrack; // IE 9/10 + if (window.navigator && window.navigator.doNotTrack !== undefined) { + flag = window.navigator.doNotTrack; // FF, Chrome + } else if (window.navigator && window.navigator.msDoNotTrack !== undefined) { + flag = window.navigator.msDoNotTrack; // IE 9/10 } else { flag = window.doNotTrack; // IE 11+, Safari } @@ -460,7 +460,7 @@ export function initialize(env, user, options = {}) { } else if ( typeof options.bootstrap === 'string' && options.bootstrap.toUpperCase() === 'LOCALSTORAGE' && - !!localStorage + !!window.localStorage ) { useLocalStorage = true; @@ -520,11 +520,11 @@ export function initialize(env, user, options = {}) { } function watchLocation(interval, callback) { - let previousUrl = location.href; + let previousUrl = window.location.href; let currentUrl; function checkUrl() { - currentUrl = location.href; + currentUrl = window.location.href; if (currentUrl !== previousUrl) { previousUrl = currentUrl; @@ -541,7 +541,7 @@ export function initialize(env, user, options = {}) { poll(checkUrl, interval); - if (!!(window.history && history.pushState)) { + if (!!(window.history && window.history.pushState)) { window.addEventListener('popstate', checkUrl); } else { window.addEventListener('hashchange', checkUrl); diff --git a/src/messages.js b/src/messages.js index 772c9c4c..fc472f6a 100644 --- a/src/messages.js +++ b/src/messages.js @@ -1,3 +1,5 @@ +import * as errors from './errors'; + const docLink = ' Please see https://docs.launchdarkly.com/docs/js-sdk-reference#section-initializing-the-client for instructions on SDK initialization.'; From fdd57e4466bfc84f08f18465d744d8bdffebc6b3 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 9 Nov 2018 13:20:52 -0800 Subject: [PATCH 02/20] fix TypeScript def for event name and add documentation --- typings.d.ts | 59 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/typings.d.ts b/typings.d.ts index 9d6ba3cf..af774b7f 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -18,11 +18,6 @@ declare module 'ldclient-js' { export default LaunchDarkly; - /** - * The names of events to which users of the client can subscribe. - */ - export type LDEventName = 'ready' | 'change'; - /** * The types of values a feature flag can have. * @@ -48,12 +43,42 @@ declare module 'ldclient-js' { }; /** - * The parameters required to (un)subscribe to/from LaunchDarkly events. + * The parameters required to (un)subscribe to/from LaunchDarkly events. See + * LDClient#on and LDClient#off. + * + * The following event names (keys) are used by the cliet: + * + * "ready": The client has finished starting up. This event will be sent regardless + * of whether it successfully connected to LaunchDarkly, or encountered an error + * and had to give up; to distinguish between these cases, see below. + * + * "initialized": The client successfully started up and has valid feature flag + * data. This will always be accompanied by "ready". + * + * "failed": The client encountered an error that prevented it from connecting to + * LaunchDarkly, such as an invalid environment ID. This will always be accompanied + * by "ready". + * + * "error": General event for any kind of error condition during client operation. + * The callback parameter is an Error object. If you do not listen for "error" + * events, then the errors will be logged with console.log(). + * + * "change": The client has received new feature flag data. This can happen either + * because you have switched users with identify(), or because the client has a + * stream connection and has received a live change to a flag value (see below). + * The callback parameter is an LDFlagChangeset. + * + * "change:FLAG-KEY": The client has received a new value for a specific flag + * whose key is FLAG-KEY. The callback receives two parameters: the current (new) + * flag value, and the previous value. This is always accompanied by a general + * "change" event as described above; you can listen for either or both. * - * See LDClient#on and LDClient#off. + * The "change" and "change:FLAG-KEY" events have special behavior: the client + * will open a streaming connection to receive live changes if and only if you + * are listening for one of these events. */ type LDEventSignature = ( - key: LDEventName, + key: string, callback: (current?: LDFlagValue | LDFlagChangeset, previous?: LDFlagValue) => void, context?: any ) => void; @@ -400,28 +425,24 @@ declare module 'ldclient-js' { variationDetail: (key: string, defaultValue?: LDFlagValue) => LDEvaluationDetail; /** - * Registers an event listener. + * Registers an event listener. See LDEventSignature for the available event types + * and the data that can be associated with them. * * @param key - * The name of the event for which to listen. This can be "ready", - * "change", or "change:FLAG-KEY". + * The name of the event for which to listen. * @param callback - * The function to execute when the event fires. For the "change" - * event, the callback receives one parameter: an LDFlagChangeset - * describing the changes. For "change:FLAG-KEY" events, the callback - * receives two parameters: the current (new) value and the previous - * value of the relevant flag. + * The function to execute when the event fires. The callback may or may not + * receive parameters, depending on the type of event; see LDEventSignature. * @param context * The "this" context to use for the callback. */ on: LDEventSignature; /** - * Deregisters an event listener. + * Deregisters an event listener. See LDEventSignature for the available event types. * * @param key - * The name of the event for which to stop listening. This can be - * "ready", "change", or "change:FLAG-KEY". + * The name of the event for which to stop listening. * @param callback * The function to deregister. * @param context From 6b43e9adbf86df50d5600504c645dffe2c59e4e4 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 9 Nov 2018 13:23:10 -0800 Subject: [PATCH 03/20] comment edit --- typings.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/typings.d.ts b/typings.d.ts index af774b7f..efebf536 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -56,8 +56,8 @@ declare module 'ldclient-js' { * data. This will always be accompanied by "ready". * * "failed": The client encountered an error that prevented it from connecting to - * LaunchDarkly, such as an invalid environment ID. This will always be accompanied - * by "ready". + * LaunchDarkly, such as an invalid environment ID. All flag evaluations will + * therefore receive default values. This will always be accompanied by "ready". * * "error": General event for any kind of error condition during client operation. * The callback parameter is an Error object. If you do not listen for "error" From 65e44c16beba441ea38fce9825d8d26bff8a0801 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 20 Nov 2018 16:14:52 -0800 Subject: [PATCH 04/20] replace Base64 dependency with a package that has a lowercase name --- package-lock.json | 10 +++++----- package.json | 4 ++-- src/__tests__/EventSender-test.js | 4 ++-- src/utils.js | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4009ca3b..ce506020 100644 --- a/package-lock.json +++ b/package-lock.json @@ -233,11 +233,6 @@ "integrity": "sha512-F/v7t1LwS4vnXuPooJQGBRKRGIoxWUTmA4VHfqjOccFsNDThD5bfUNpITive6s352O7o384wcpEaDV8rHCehDA==", "dev": true }, - "Base64": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/Base64/-/Base64-1.0.1.tgz", - "integrity": "sha1-3vRcxQyWG8yb8jIdD1K8v+wfG7E=" - }, "abab": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/abab/-/abab-1.0.4.tgz", @@ -5891,6 +5886,11 @@ "merge-stream": "^1.0.1" } }, + "js-base64": { + "version": "2.4.9", + "resolved": "https://registry.npmjs.org/js-base64/-/js-base64-2.4.9.tgz", + "integrity": "sha512-xcinL3AuDJk7VSzsHgb9DvvIXayBbadtMZ4HFPx8rUszbW1MuNMlwYVC4zzCZ6e1sqZpnNS5ZFYOhXqA39T7LQ==" + }, "js-tokens": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-3.0.2.tgz", diff --git a/package.json b/package.json index 7c65fc23..b9497c86 100755 --- a/package.json +++ b/package.json @@ -78,8 +78,8 @@ "typescript": "3.0.1" }, "dependencies": { - "Base64": "1.0.1", - "escape-string-regexp": "1.0.5" + "escape-string-regexp": "1.0.5", + "js-base64": "2.4.9" }, "repository": { "type": "git", diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index ca5ac683..670b78f6 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -1,4 +1,4 @@ -import Base64 from 'Base64'; +import { Base64 } from 'js-base64'; import sinon from 'sinon'; import EventSender from '../EventSender'; @@ -44,7 +44,7 @@ describe('EventSender', () => { s = s + '='; } s = s.replace(/_/g, '/').replace(/-/g, '+'); - return decodeURIComponent(escape(Base64.atob(s))); + return decodeURIComponent(escape(Base64.decode(s))); } function decodeOutputFromUrl(url) { diff --git a/src/utils.js b/src/utils.js index c9edeaba..beec17b5 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,8 +1,8 @@ -import Base64 from 'Base64'; +import { Base64 } from 'js-base64'; // See http://ecmanaut.blogspot.com/2006/07/encoding-decoding-utf8-in-javascript.html export function btoa(s) { - return Base64.btoa(unescape(encodeURIComponent(s))); + return Base64.encode(unescape(encodeURIComponent(s))); } export function base64URLEncode(s) { From 6baa06e3a4bc2d7669b136003823e9cc1801ea26 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 20 Nov 2018 16:43:53 -0800 Subject: [PATCH 05/20] use a different package due to import problems --- package-lock.json | 10 +++++----- package.json | 4 ++-- src/__tests__/EventSender-test.js | 6 ++++-- src/utils.js | 13 +++++++++++-- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index ce506020..736da68a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1486,6 +1486,11 @@ } } }, + "base64-js": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.3.0.tgz", + "integrity": "sha512-ccav/yGvoa80BQDljCxsmmQ3Xvx60/UpBIij5QN21W3wBi/hhIC9OoO+KLpu9IJTS9j4DRVJ3aDDF9cMSoa2lw==" + }, "bcrypt-pbkdf": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/bcrypt-pbkdf/-/bcrypt-pbkdf-1.0.2.tgz", @@ -5886,11 +5891,6 @@ "merge-stream": "^1.0.1" } }, - "js-base64": { - "version": "2.4.9", - "resolved": "https://registry.npmjs.org/js-base64/-/js-base64-2.4.9.tgz", - "integrity": "sha512-xcinL3AuDJk7VSzsHgb9DvvIXayBbadtMZ4HFPx8rUszbW1MuNMlwYVC4zzCZ6e1sqZpnNS5ZFYOhXqA39T7LQ==" - }, "js-tokens": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-3.0.2.tgz", diff --git a/package.json b/package.json index b9497c86..d1c6c00f 100755 --- a/package.json +++ b/package.json @@ -78,8 +78,8 @@ "typescript": "3.0.1" }, "dependencies": { - "escape-string-regexp": "1.0.5", - "js-base64": "2.4.9" + "base64-js": "1.3.0", + "escape-string-regexp": "1.0.5" }, "repository": { "type": "git", diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index 670b78f6..c130de76 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -1,4 +1,4 @@ -import { Base64 } from 'js-base64'; +import * as base64 from 'base64-js'; import sinon from 'sinon'; import EventSender from '../EventSender'; @@ -44,7 +44,9 @@ describe('EventSender', () => { s = s + '='; } s = s.replace(/_/g, '/').replace(/-/g, '+'); - return decodeURIComponent(escape(Base64.decode(s))); + const decodedBytes = base64.toByteArray(s); + const decodedStr = String.fromCharCode.apply(String, decodedBytes); + return decodeURIComponent(escape(decodedStr)); } function decodeOutputFromUrl(url) { diff --git a/src/utils.js b/src/utils.js index beec17b5..98d0f2cc 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,8 +1,17 @@ -import { Base64 } from 'js-base64'; +import * as base64 from 'base64-js'; // See http://ecmanaut.blogspot.com/2006/07/encoding-decoding-utf8-in-javascript.html export function btoa(s) { - return Base64.encode(unescape(encodeURIComponent(s))); + const escaped = unescape(encodeURIComponent(s)); + return base64.fromByteArray(stringToBytes(escaped)); +} + +function stringToBytes(s) { + var b = []; + for (var i = 0; i < s.length; i++) { + b.push(s.charCodeAt(i)); + } + return b; } export function base64URLEncode(s) { From d5228b683292c928c2004626e7bff4b96b0eca3a Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 20 Nov 2018 16:45:45 -0800 Subject: [PATCH 06/20] linter --- src/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils.js b/src/utils.js index 98d0f2cc..c716232a 100644 --- a/src/utils.js +++ b/src/utils.js @@ -7,8 +7,8 @@ export function btoa(s) { } function stringToBytes(s) { - var b = []; - for (var i = 0; i < s.length; i++) { + const b = []; + for (let i = 0; i < s.length; i++) { b.push(s.charCodeAt(i)); } return b; From c1ef0cb2243d26a1a4d993ed2a45079cb46d8e83 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 20 Nov 2018 17:11:41 -0800 Subject: [PATCH 07/20] override short default timeout in one EventSource polyfill --- src/Stream.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Stream.js b/src/Stream.js index 07548c1f..d993554e 100644 --- a/src/Stream.js +++ b/src/Stream.js @@ -62,7 +62,17 @@ export default function Stream(baseUrl, environment, hash, config) { url = url + (query ? '?' : '') + query; closeConnection(); - es = new window.EventSource(url); + + // The standard EventSource constructor doesn't take any options, just a URL. However, there's + // a known issue with one of the EventSource polyfills, Yaffle, which has a fairly short + // default timeout - much shorter than our heartbeat interval - causing unnecessary reconnect + // attempts and error logging. Yaffle allows us to override this with the "heartbeatTimeout" + // property. This should be ignored by other implementations that don't have such an option. + const options = { + heartbeatTimeout: 300000 // 5-minute timeout; LD stream sends heartbeats every 3 min + }; + + es = new window.EventSource(url, options); for (const key in handlers) { if (handlers.hasOwnProperty(key)) { es.addEventListener(key, handlers[key]); From 6bdeb156ef95f1fb68713e57a7716021ed86170f Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 20 Nov 2018 17:13:51 -0800 Subject: [PATCH 08/20] linter --- src/Stream.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Stream.js b/src/Stream.js index d993554e..4ad9e510 100644 --- a/src/Stream.js +++ b/src/Stream.js @@ -69,7 +69,7 @@ export default function Stream(baseUrl, environment, hash, config) { // attempts and error logging. Yaffle allows us to override this with the "heartbeatTimeout" // property. This should be ignored by other implementations that don't have such an option. const options = { - heartbeatTimeout: 300000 // 5-minute timeout; LD stream sends heartbeats every 3 min + heartbeatTimeout: 300000, // 5-minute timeout; LD stream sends heartbeats every 3 min }; es = new window.EventSource(url, options); From 7246704cdd21272ca696b64011ba5eed0c12a90f Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 21 Nov 2018 16:10:21 -0800 Subject: [PATCH 09/20] misc fixes --- README.md | 2 +- src/Stream.js | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e2214596..e1216d72 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ when `client.on('change')` is called. If you need streaming support, and you wish to support browsers that do not support `EventSource` natively, you can install a polyfill such as -[EventSource](https://github.com/Yaffle/EventSource). +[event-source-polyfill](https://github.com/Yaffle/EventSource). #### CDN diff --git a/src/Stream.js b/src/Stream.js index 4ad9e510..8476c4a7 100644 --- a/src/Stream.js +++ b/src/Stream.js @@ -6,6 +6,7 @@ export default function Stream(baseUrl, environment, hash, config) { const useReport = (config && config.useReport) || false; const withReasons = (config && config.evaluationReasons) || false; const streamReconnectDelay = (config && config.streamReconnectDelay) || 1000; + const timeoutMillis = 300000; // 5 minutes (same as other SDKs) - note, this only has an effect on polyfills let es = null; let reconnectTimeoutReference = null; let user = null; @@ -63,13 +64,13 @@ export default function Stream(baseUrl, environment, hash, config) { closeConnection(); - // The standard EventSource constructor doesn't take any options, just a URL. However, there's - // a known issue with one of the EventSource polyfills, Yaffle, which has a fairly short - // default timeout - much shorter than our heartbeat interval - causing unnecessary reconnect - // attempts and error logging. Yaffle allows us to override this with the "heartbeatTimeout" - // property. This should be ignored by other implementations that don't have such an option. + // The standard EventSource constructor doesn't take any options, just a URL. However, some + // EventSource polyfills allow us to specify a timeout interval, and in some cases they will + // default to a too-short timeout if we don't specify one. So, here, we are setting the + // timeout properties that are used by several popular polyfills. const options = { - heartbeatTimeout: 300000, // 5-minute timeout; LD stream sends heartbeats every 3 min + heartbeatTimeout: timeoutMillis, // used by "event-source-polyfill" package + silentTimeout: timeoutMillis, // used by "eventsource-polyfill" package }; es = new window.EventSource(url, options); From 0786a481774ea014981dac0bafea241a1c8c2286 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Nov 2018 13:43:31 -0800 Subject: [PATCH 10/20] allow streaming mode to be decoupled from event subscription --- src/EventEmitter.js | 8 ++ src/__tests__/LDClient-streaming-test.js | 154 ++++++++++++++++++++--- src/index.js | 46 ++++++- typings.d.ts | 29 ++++- 4 files changed, 209 insertions(+), 28 deletions(-) diff --git a/src/EventEmitter.js b/src/EventEmitter.js index 63484972..49857293 100644 --- a/src/EventEmitter.js +++ b/src/EventEmitter.js @@ -32,6 +32,14 @@ export default function EventEmitter() { } }; + emitter.getEvents = function() { + return Object.keys(events); + }; + + emitter.getEventListenerCount = function(event) { + return events[event] ? events[event].length : 0; + }; + emitter.maybeReportError = function(error) { if (!error) { return; diff --git a/src/__tests__/LDClient-streaming-test.js b/src/__tests__/LDClient-streaming-test.js index 5904d190..dee5da44 100644 --- a/src/__tests__/LDClient-streaming-test.js +++ b/src/__tests__/LDClient-streaming-test.js @@ -44,37 +44,157 @@ describe('LDClient', () => { describe('streaming/event listening', () => { const streamUrl = 'https://clientstream.launchdarkly.com'; + const fullStreamUrlWithUser = streamUrl + '/eval/' + envName + '/' + encodedUser; function streamEvents() { - return sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events; + return sources[fullStreamUrlWithUser].__emitter._events; + } + + function expectStreamUrlIsOpen(url) { + expect(Object.keys(sources)).toEqual([url]); + } + + function expectNoStreamIsOpen() { + expect(sources).toMatchObject({}); } it('does not connect to the stream by default', done => { const client = LDClient.initialize(envName, user, { bootstrap: {} }); client.on('ready', () => { - expect(sources).toMatchObject({}); + expectNoStreamIsOpen(); done(); }); }); - it('connects to the stream when listening to global change events', done => { - const client = LDClient.initialize(envName, user, { bootstrap: {} }); + it('connects to the stream if options.streaming is true', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {}, streaming: true }); client.on('ready', () => { - client.on('change', () => {}); - expect(Object.keys(sources)).toEqual([streamUrl + '/eval/' + envName + '/' + encodedUser]); + expectStreamUrlIsOpen(fullStreamUrlWithUser); done(); }); }); - it('connects to the stream when listening to change event for one flag', done => { - const client = LDClient.initialize(envName, user, { bootstrap: {} }); + describe('setStreaming()', () => { + it('can connect to the stream', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); - client.on('ready', () => { - client.on('change:flagkey', () => {}); - expect(Object.keys(sources)).toEqual([streamUrl + '/eval/' + envName + '/' + encodedUser]); - done(); + client.on('ready', () => { + client.setStreaming(true); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + done(); + }); + }); + + it('can disconnect from the stream', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.setStreaming(true); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + client.setStreaming(false); + expectNoStreamIsOpen(); + done(); + }); + }); + }); + + describe('on("change")', () => { + it('connects to the stream if not otherwise overridden', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.on('change', () => {}); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + done(); + }); + }); + + it('also connects if listening for a specific flag', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.on('change:flagkey', () => {}); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + done(); + }); + }); + + it('does not connect if some other kind of event was specified', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.on('error', () => {}); + expectNoStreamIsOpen(); + done(); + }); + }); + + it('does not connect if options.streaming is explicitly set to false', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {}, streaming: false }); + + client.on('ready', () => { + client.on('change', () => {}); + expectNoStreamIsOpen(); + done(); + }); + }); + + it('does not connect if setStreaming(false) was called', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.setStreaming(false); + client.on('change', () => {}); + expectNoStreamIsOpen(); + done(); + }); + }); + }); + + describe('off("change")', () => { + it('disconnects from the stream if all event listeners are removed', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + const listener1 = () => {}; + const listener2 = () => {}; + + client.on('ready', () => { + client.on('change', listener1); + client.on('change:flagkey', listener2); + client.on('error', () => {}); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + + client.off('change', listener1); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + + client.off('change:flagkey', listener2); + expectNoStreamIsOpen(); + + done(); + }); + }); + + it('does not disconnect if setStreaming(true) was called', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + const listener1 = () => {}; + const listener2 = () => {}; + + client.on('ready', () => { + client.setStreaming(true); + + client.on('change', listener1); + client.on('change:flagkey', listener2); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + + client.off('change', listener1); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + + client.off('change:flagkey', listener2); + expectStreamUrlIsOpen(fullStreamUrlWithUser); + + done(); + }); }); }); @@ -83,7 +203,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change:flagkey', () => {}); - expect(Object.keys(sources)).toEqual([streamUrl + '/eval/' + envName + '/' + encodedUser + '?h=' + hash]); + expectStreamUrlIsOpen(fullStreamUrlWithUser + '?h=' + hash); done(); }); }); @@ -93,9 +213,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - expect(Object.keys(sources)).toEqual([ - streamUrl + '/eval/' + envName + '/' + encodedUser + '?withReasons=true', - ]); + expectStreamUrlIsOpen(fullStreamUrlWithUser + '?withReasons=true'); done(); }); }); @@ -105,9 +223,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - expect(Object.keys(sources)).toEqual([ - streamUrl + '/eval/' + envName + '/' + encodedUser + '?h=' + hash + '&withReasons=true', - ]); + expectStreamUrlIsOpen(fullStreamUrlWithUser + '?h=' + hash + '&withReasons=true'); done(); }); }); diff --git a/src/index.js b/src/index.js index 09a32950..66aebf9e 100644 --- a/src/index.js +++ b/src/index.js @@ -36,6 +36,8 @@ export function initialize(env, user, options = {}) { let goalTracker; let useLocalStorage; let goals; + let streamActive; + let streamForcedState; let subscribedToChangeEvents; let firstEvent = true; @@ -180,7 +182,7 @@ export function initialize(env, user, options = {}) { updateSettings(settings); } resolve(utils.transformVersionedValuesToValues(settings)); - if (subscribedToChangeEvents) { + if (streamActive) { connectStream(); } }); @@ -299,6 +301,7 @@ export function initialize(env, user, options = {}) { } function connectStream() { + streamActive = true; if (!ident.getUser()) { return; } @@ -403,9 +406,9 @@ export function initialize(env, user, options = {}) { } function on(event, handler, context) { - if (event.substr(0, changeEvent.length) === changeEvent) { + if (isChangeEventKey(event)) { subscribedToChangeEvents = true; - if (!stream.isConnected()) { + if (!streamActive && streamForcedState === undefined) { connectStream(); } emitter.on.apply(emitter, [event, handler, context]); @@ -415,13 +418,40 @@ export function initialize(env, user, options = {}) { } function off(event) { - if (event === changeEvent) { - if ((subscribedToChangeEvents = true)) { + emitter.off.apply(emitter, Array.prototype.slice.call(arguments)); + if (isChangeEventKey(event)) { + let haveListeners = false; + emitter.getEvents().forEach(key => { + if (isChangeEventKey(key) && emitter.getEventListenerCount(key) > 0) { + haveListeners = true; + } + }); + if (!haveListeners) { subscribedToChangeEvents = false; + if (streamActive && streamForcedState === undefined) { + streamActive = false; + stream.disconnect(); + } + } + } + } + + function setStreaming(state) { + const newState = state === null ? undefined : state; + if (newState !== streamForcedState) { + streamForcedState = newState; + let shouldBeStreaming = streamForcedState || (subscribedToChangeEvents && streamForcedState === undefined); + if (shouldBeStreaming && !streamActive) { + connectStream(); + } else if (!shouldBeStreaming && streamActive) { + streamActive = false; stream.disconnect(); } } - emitter.off.apply(emitter, Array.prototype.slice.call(arguments)); + } + + function isChangeEventKey(event) { + return event === changeEvent || event.substr(0, changeEvent.length + 1) === changeEvent + ':'; } function handleMessage(event) { @@ -565,6 +595,9 @@ export function initialize(env, user, options = {}) { } function signalSuccessfulInit() { + if (options.streaming !== undefined) { + setStreaming(options.streaming); + } emitter.emit(readyEvent); emitter.emit(successEvent); // allows initPromise to distinguish between success and failure } @@ -631,6 +664,7 @@ export function initialize(env, user, options = {}) { track: track, on: on, off: off, + setStreaming: setStreaming, flush: flush, allFlags: allFlags, }; diff --git a/typings.d.ts b/typings.d.ts index efebf536..674d9332 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -73,9 +73,10 @@ declare module 'ldclient-js' { * flag value, and the previous value. This is always accompanied by a general * "change" event as described above; you can listen for either or both. * - * The "change" and "change:FLAG-KEY" events have special behavior: the client - * will open a streaming connection to receive live changes if and only if you - * are listening for one of these events. + * The "change" and "change:FLAG-KEY" events have special behavior: by default, the + * client will open a streaming connection to receive live changes if and only if + * you are listening for one of these events. This behavior can be overridden by + * setting LDOptions.streaming or calling LDClient.setStreaming(). */ type LDEventSignature = ( key: string, @@ -128,6 +129,17 @@ declare module 'ldclient-js' { */ streamUrl?: string; + /** + * Whether or not to open a streaming connection to LaunchDarkly for live flag updates. + * + * If this is true, the client will always attempt to maintain a streaming connection; if false, + * it never will. If you leave the value undefined (the default), the client will open a streaming + * connection if you subscribe to "change" or "change:flag-key" events (see LDClient.on()). + * + * This is equivalent to calling client.setStreaming() with the same value. + */ + streaming?: boolean; + /** * Whether or not to use the REPORT verb to fetch flag settings. * @@ -424,6 +436,17 @@ declare module 'ldclient-js' { */ variationDetail: (key: string, defaultValue?: LDFlagValue) => LDEvaluationDetail; + /** + * Specifies whether or not to open a streaming connection to LaunchDarkly for live flag updates. + * + * If this is true, the client will always attempt to maintain a streaming connection; if false, + * it never will. If you leave the value undefined (the default), the client will open a streaming + * connection if you subscribe to "change" or "change:flag-key" events (see LDClient.on()). + * + * This can also be set as the "streaming" property of the client options. + */ + setStreaming: (value?: boolean) => void; + /** * Registers an event listener. See LDEventSignature for the available event types * and the data that can be associated with them. From bb291944cdc542a475d8f957c65ba2decd1d4373 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Sat, 24 Nov 2018 13:49:57 -0800 Subject: [PATCH 11/20] misc cleanup --- src/index.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/index.js b/src/index.js index 66aebf9e..1a18613f 100644 --- a/src/index.js +++ b/src/index.js @@ -352,6 +352,13 @@ export function initialize(env, user, options = {}) { }); } + function disconnectStream() { + if (streamActive) { + stream.disconnect(); + streamActive = false; + } + } + function updateSettings(newFlags) { const changes = {}; @@ -429,8 +436,7 @@ export function initialize(env, user, options = {}) { if (!haveListeners) { subscribedToChangeEvents = false; if (streamActive && streamForcedState === undefined) { - streamActive = false; - stream.disconnect(); + disconnectStream(); } } } @@ -440,12 +446,11 @@ export function initialize(env, user, options = {}) { const newState = state === null ? undefined : state; if (newState !== streamForcedState) { streamForcedState = newState; - let shouldBeStreaming = streamForcedState || (subscribedToChangeEvents && streamForcedState === undefined); + const shouldBeStreaming = streamForcedState || (subscribedToChangeEvents && streamForcedState === undefined); if (shouldBeStreaming && !streamActive) { connectStream(); } else if (!shouldBeStreaming && streamActive) { - streamActive = false; - stream.disconnect(); + disconnectStream(); } } } From 624afa8e30da4aa560a14f225f9a1906f8bc046c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 27 Nov 2018 11:27:45 -0800 Subject: [PATCH 12/20] expand unit test to make sure off() really works --- src/__tests__/LDClient-streaming-test.js | 36 ++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/__tests__/LDClient-streaming-test.js b/src/__tests__/LDClient-streaming-test.js index dee5da44..50b599e6 100644 --- a/src/__tests__/LDClient-streaming-test.js +++ b/src/__tests__/LDClient-streaming-test.js @@ -175,27 +175,53 @@ describe('LDClient', () => { }); }); - it('does not disconnect if setStreaming(true) was called', done => { + it('does not disconnect if setStreaming(true) was called, but still removes event listener', done => { + const changes1 = []; + const changes2 = []; + const client = LDClient.initialize(envName, user, { bootstrap: {} }); - const listener1 = () => {}; - const listener2 = () => {}; + const listener1 = allValues => changes1.push(allValues); + const listener2 = newValue => changes2.push(newValue); client.on('ready', () => { client.setStreaming(true); client.on('change', listener1); - client.on('change:flagkey', listener2); + client.on('change:flag', listener2); expectStreamUrlIsOpen(fullStreamUrlWithUser); + streamEvents().put({ + data: '{"flag":{"value":"a","version":1}}', + }); + + expect(changes1).toEqual([{flag: {current: "a", previous:undefined}}]); + expect(changes2).toEqual(["a"]); + client.off('change', listener1); expectStreamUrlIsOpen(fullStreamUrlWithUser); - client.off('change:flagkey', listener2); + streamEvents().put({ + data: '{"flag":{"value":"b","version":1}}', + }); + + expect(changes1).toEqual([{flag: {current: "a", previous:undefined}}]); + expect(changes2).toEqual(["a", "b"]); + + client.off('change:flag', listener2); expectStreamUrlIsOpen(fullStreamUrlWithUser); + streamEvents().put({ + data: '{"flag":{"value":"c","version":1}}', + }); + + expect(changes1).toEqual([{flag: {current: "a", previous:undefined}}]); + expect(changes2).toEqual(["a", "b"]); + done(); }); }); + + }); it('passes the secure mode hash in the stream URL if provided', done => { From 24bb4c40c4d32b2dff1cccda63d10f810c3c0a9c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 27 Nov 2018 11:32:28 -0800 Subject: [PATCH 13/20] linter --- src/__tests__/LDClient-streaming-test.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/__tests__/LDClient-streaming-test.js b/src/__tests__/LDClient-streaming-test.js index 50b599e6..3355aea3 100644 --- a/src/__tests__/LDClient-streaming-test.js +++ b/src/__tests__/LDClient-streaming-test.js @@ -194,8 +194,8 @@ describe('LDClient', () => { data: '{"flag":{"value":"a","version":1}}', }); - expect(changes1).toEqual([{flag: {current: "a", previous:undefined}}]); - expect(changes2).toEqual(["a"]); + expect(changes1).toEqual([{ flag: { current: 'a', previous: undefined } }]); + expect(changes2).toEqual(['a']); client.off('change', listener1); expectStreamUrlIsOpen(fullStreamUrlWithUser); @@ -204,8 +204,8 @@ describe('LDClient', () => { data: '{"flag":{"value":"b","version":1}}', }); - expect(changes1).toEqual([{flag: {current: "a", previous:undefined}}]); - expect(changes2).toEqual(["a", "b"]); + expect(changes1).toEqual([{ flag: { current: 'a', previous: undefined } }]); + expect(changes2).toEqual(['a', 'b']); client.off('change:flag', listener2); expectStreamUrlIsOpen(fullStreamUrlWithUser); @@ -214,14 +214,12 @@ describe('LDClient', () => { data: '{"flag":{"value":"c","version":1}}', }); - expect(changes1).toEqual([{flag: {current: "a", previous:undefined}}]); - expect(changes2).toEqual(["a", "b"]); + expect(changes1).toEqual([{ flag: { current: 'a', previous: undefined } }]); + expect(changes2).toEqual(['a', 'b']); done(); }); }); - - }); it('passes the secure mode hash in the stream URL if provided', done => { From 4bba83f23250983654a9f27ccab0141723fc96a2 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 27 Nov 2018 11:36:56 -0800 Subject: [PATCH 14/20] simplify code using spread operator --- src/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index 1a18613f..19cabdb3 100644 --- a/src/index.js +++ b/src/index.js @@ -418,14 +418,14 @@ export function initialize(env, user, options = {}) { if (!streamActive && streamForcedState === undefined) { connectStream(); } - emitter.on.apply(emitter, [event, handler, context]); + emitter.on(event, handler, context); } else { - emitter.on.apply(emitter, Array.prototype.slice.call(arguments)); + emitter.on(...arguments); } } function off(event) { - emitter.off.apply(emitter, Array.prototype.slice.call(arguments)); + emitter.off(...arguments); if (isChangeEventKey(event)) { let haveListeners = false; emitter.getEvents().forEach(key => { From 525ca376bd95a4dadfe3e6df24c8b8ff22047258 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 27 Nov 2018 12:32:00 -0800 Subject: [PATCH 15/20] fire change event when updating flags after bootstrap from localstorage --- src/__tests__/LDClient-test.js | 19 +++++++++++++++++++ src/index.js | 11 +++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 1688a160..723407fd 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -217,6 +217,25 @@ describe('LDClient', () => { .catch(() => {}); }); + it('should load flags from local storage and then request newer ones', done => { + const json = '{"flag": "a"}'; + + window.localStorage.setItem(lsKey, json); + + const client = LDClient.initialize(envName, user, { bootstrap: 'localstorage', streaming: false }); + + client.waitForInitialization().then(() => { + expect(client.variation('flag')).toEqual('a'); + + client.on('change:flag', newValue => { + expect(newValue).toEqual('b'); + done(); + }); + + requests[0].respond(200, { 'Content-Type': 'application/json' }, '{"flag": {"value": "b", "version": 2}}'); + }); + }); + it('should start with empty flags if we tried to use cached settings and there are none', done => { window.localStorage.removeItem(lsKey); diff --git a/src/index.js b/src/index.js index 19cabdb3..50ef63f0 100644 --- a/src/index.js +++ b/src/index.js @@ -509,8 +509,7 @@ export function initialize(env, user, options = {}) { signalFailedInit(initErr); } else { if (settings) { - flags = settings; - store.saveFlags(flags); + updateSettings(settings); // this includes saving to local storage and sending change events } else { flags = {}; } @@ -518,9 +517,9 @@ export function initialize(env, user, options = {}) { } }); } else { - // We're reading the flags from local storage. Signal that we're ready, - // then update localStorage for the next page load. We won't signal changes or update - // the in-memory flags unless you subscribe for changes + // We're reading the flags from local storage. Signal that we're ready immediately, but also + // start a request in the background to get newer flags. When we receive those, we will update + // localStorage, and will also send change events if the values have changed. utils.onNextTick(signalSuccessfulInit); requestor.fetchFlagSettings(ident.getUser(), hash, (err, settings) => { @@ -528,7 +527,7 @@ export function initialize(env, user, options = {}) { emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); } if (settings) { - store.saveFlags(settings); + updateSettings(settings); // this includes saving to local storage and sending change events } }); } From af0fcfc91bcd95fefa86431f57de34f75950ee74 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 27 Nov 2018 12:33:56 -0800 Subject: [PATCH 16/20] linter --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 50ef63f0..4c51fe12 100644 --- a/src/index.js +++ b/src/index.js @@ -527,7 +527,7 @@ export function initialize(env, user, options = {}) { emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); } if (settings) { - updateSettings(settings); // this includes saving to local storage and sending change events + updateSettings(settings); // this includes saving to local storage and sending change events } }); } From 41b1b2e7605bc0eab76f5e351d45986c5817b803 Mon Sep 17 00:00:00 2001 From: Jay Date: Wed, 28 Nov 2018 16:12:06 -0800 Subject: [PATCH 17/20] fix typos --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e1216d72..8c223466 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ The LaunchDarkly client-side JavaScript SDK supports the following browsers: * Chrome (any recent) * Firefox (any recent) -* Safari (any recent)\* +* Safari (any recent) * Internet Explorer (IE10+)\* * Edge (any recent)\* * Opera (any recent)\* @@ -67,7 +67,7 @@ Then import it before the module that initializes the LaunchDarkly client: ### Promise polyfill -The newer versions of the use `Promise`. If you need to support older browsers, you will +Newer versions of the SDK use `Promise`. If you need to support older browsers, you will need to install a polyfill for it, such as [es6-promise](https://github.com/stefanpenner/es6-promise). #### CDN From a4e07d4d442c0552029cd5e03f6350178ccdbf5f Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 28 Nov 2018 18:39:20 -0800 Subject: [PATCH 18/20] use deep compare to decide whether a flag value has changed --- package-lock.json | 15 ++++++++++---- package.json | 3 ++- src/__tests__/LDClient-streaming-test.js | 25 ++++++++++++++++++++++++ src/index.js | 2 +- src/utils.js | 5 +++++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index dad90f21..7583f683 100644 --- a/package-lock.json +++ b/package-lock.json @@ -322,6 +322,14 @@ "fast-deep-equal": "^1.0.0", "fast-json-stable-stringify": "^2.0.0", "json-schema-traverse": "^0.3.0" + }, + "dependencies": { + "fast-deep-equal": { + "version": "1.1.0", + "resolved": "http://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-1.1.0.tgz", + "integrity": "sha1-wFNHeBfIa1HaqFPIHgWbcz0CNhQ=", + "dev": true + } } }, "ajv-keywords": { @@ -2962,10 +2970,9 @@ "dev": true }, "fast-deep-equal": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-1.0.0.tgz", - "integrity": "sha1-liVqO8l1WV6zbYLpkp0GDYk0Of8=", - "dev": true + "version": "2.0.1", + "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz", + "integrity": "sha1-ewUhjd+WZ79/Nwv3/bLLFf3Qqkk=" }, "fast-diff": { "version": "1.1.2", diff --git a/package.json b/package.json index 80ac2cb3..bffafa0c 100755 --- a/package.json +++ b/package.json @@ -79,7 +79,8 @@ }, "dependencies": { "base64-js": "1.3.0", - "escape-string-regexp": "1.0.5" + "escape-string-regexp": "1.0.5", + "fast-deep-equal": "2.0.1" }, "repository": { "type": "git", diff --git a/src/__tests__/LDClient-streaming-test.js b/src/__tests__/LDClient-streaming-test.js index 3355aea3..8b00085b 100644 --- a/src/__tests__/LDClient-streaming-test.js +++ b/src/__tests__/LDClient-streaming-test.js @@ -321,6 +321,31 @@ describe('LDClient', () => { }); }); + it('does not fire change event if new and old values are equivalent JSON objects', done => { + const client = LDClient.initialize(envName, user, { + bootstrap: { + 'will-change': 3, + 'wont-change': { a: 1, b: 2 }, + }, + }); + + client.on('ready', () => { + client.on('change', changes => { + expect(changes).toEqual({ + 'will-change': { current: 4, previous: 3 }, + }); + + done(); + }); + + const putData = { + 'will-change': { value: 4, version: 2 }, + 'wont-change': { value: { b: 2, a: 1 }, version: 2 }, + }; + streamEvents().put({ data: JSON.stringify(putData) }); + }); + }); + it('fires individual change event when flags are updated from put event', done => { const client = LDClient.initialize(envName, user, { bootstrap: { 'enable-foo': false } }); diff --git a/src/index.js b/src/index.js index 4c51fe12..c7b0516c 100644 --- a/src/index.js +++ b/src/index.js @@ -368,7 +368,7 @@ export function initialize(env, user, options = {}) { for (const key in flags) { if (flags.hasOwnProperty(key) && flags[key]) { - if (newFlags[key] && newFlags[key].value !== flags[key].value) { + if (newFlags[key] && !utils.deepEquals(newFlags[key].value, flags[key].value)) { changes[key] = { previous: flags[key].value, current: getFlagDetail(newFlags[key]) }; } else if (!newFlags[key] || newFlags[key].deleted) { changes[key] = { previous: flags[key].value }; diff --git a/src/utils.js b/src/utils.js index c716232a..0757575f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,4 +1,5 @@ import * as base64 from 'base64-js'; +import fastDeepEqual from 'fast-deep-equal'; // See http://ecmanaut.blogspot.com/2006/07/encoding-decoding-utf8-in-javascript.html export function btoa(s) { @@ -28,6 +29,10 @@ export function clone(obj) { return JSON.parse(JSON.stringify(obj)); } +export function deepEquals(a, b) { + return fastDeepEqual(a, b); +} + // Events emitted in LDClient's initialize method will happen before the consumer // can register a listener, so defer them to next tick. export function onNextTick(cb) { From 7ae2cfac23ec7bc1184076a453281f59dac68936 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 3 Dec 2018 15:18:07 -0800 Subject: [PATCH 19/20] version 2.8.0 --- CHANGELOG.md | 8 ++++++ package-lock.json | 2 +- package.json | 2 +- packages/ldclient-electron/test-types.js | 31 +++++++++++++++++++++++ packages/ldclient-js-common/test-types.js | 3 +++ packages/ldclient-js/test-types.js | 30 ++++++++++++++++++++++ 6 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 packages/ldclient-electron/test-types.js create mode 100644 packages/ldclient-js-common/test-types.js create mode 100644 packages/ldclient-js/test-types.js diff --git a/CHANGELOG.md b/CHANGELOG.md index c30cc048..651bc8bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to the LaunchDarkly client-side JavaScript SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [2.8.0] - 2018-12-03 +### Added: +- The use of a streaming connection to LaunchDarkly for receiving live updates can now be controlled with the new `client.setStreaming()` method, or the equivalent boolean `streaming` property in the client configuration. If you set this to `false`, the client will not open a streaming connection even if you subscribe to change events (you might want to do this if, for instance, you just want to be notified when the client gets new flag values due to having switched users). If you set it to `true`, the client will open a streaming connection regardless of whether you subscribe to change events or not (the flag values will simply be updated in the background). If you don't set it either way then the default behavior still applies, i.e. the client opens a streaming connection if and only if you subscribe to change events. + +### Fixed: +- If the client opened a streaming connection because you called `on('change', ...)` one or more times, it will not close the connection until you call `off()` for _all_ of your event listeners. Previously, it was closing the connection whenever `off('change')` was called, even if you still had a listener for `'change:specific-flag-key'`. +- The client's logic for signaling a `change` event was using a regular Javascript `===` comparison, so it could incorrectly decide that a flag had changed if its value was a JSON object or an array. This has been fixed to use deep equality checking for object and array values. + ## [2.7.5] - 2018-11-21 ### Fixed: - When using the [`event-source-polyfill`](https://github.com/Yaffle/EventSource) package to allow streaming mode in browsers with no native EventSource support, the polyfill was using a default read timeout of 45 seconds, so if no updates arrived within 45 seconds it would log an error and reconnect the stream. The SDK now sets its own timeout (5 minutes) which will be used if this particular polyfill is active. LaunchDarkly normally sends a heartbeat every 3 minutes, so you should not see a timeout happen unless the connection has been lost. diff --git a/package-lock.json b/package-lock.json index 7583f683..74ae3fbc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "ldclient-js", - "version": "2.7.5", + "version": "2.8.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index bffafa0c..b6e3f0d9 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ldclient-js", - "version": "2.7.5", + "version": "2.8.0", "description": "LaunchDarkly SDK for JavaScript", "author": "LaunchDarkly ", "license": "Apache-2.0", diff --git a/packages/ldclient-electron/test-types.js b/packages/ldclient-electron/test-types.js new file mode 100644 index 00000000..cebd060e --- /dev/null +++ b/packages/ldclient-electron/test-types.js @@ -0,0 +1,31 @@ +"use strict"; +// This file exists only so that we can run the TypeScript compiler in the CI build +// to validate our typings.d.ts file. +exports.__esModule = true; +var ldclient_electron_1 = require("ldclient-electron"); +var emptyOptions = {}; +var logger = ldclient_electron_1.createConsoleLogger("info"); +var allOptions = { + bootstrap: {}, + baseUrl: '', + eventsUrl: '', + streamUrl: '', + streaming: true, + useReport: true, + sendLDHeaders: true, + evaluationReasons: true, + sendEvents: true, + allAttributesPrivate: true, + privateAttributeNames: ['x'], + allowFrequentDuplicateEvents: true, + sendEventsOnlyForVariation: true, + flushInterval: 1, + samplingInterval: 1, + streamReconnectDelay: 1, + logger: logger +}; +var user = { key: 'user' }; +var client1 = ldclient_electron_1.initializeMain('env', user, allOptions); +var client2 = ldclient_electron_1.initializeInRenderer('env', allOptions); +var client2WithDefaults = ldclient_electron_1.initializeInRenderer(); +var client3 = ldclient_electron_1.createNodeSdkAdapter(client1); diff --git a/packages/ldclient-js-common/test-types.js b/packages/ldclient-js-common/test-types.js new file mode 100644 index 00000000..fdd666c0 --- /dev/null +++ b/packages/ldclient-js-common/test-types.js @@ -0,0 +1,3 @@ +"use strict"; +// This file exists only so that we can run the TypeScript compiler in the CI build +// to validate our typings.d.ts file. diff --git a/packages/ldclient-js/test-types.js b/packages/ldclient-js/test-types.js new file mode 100644 index 00000000..f1367896 --- /dev/null +++ b/packages/ldclient-js/test-types.js @@ -0,0 +1,30 @@ +"use strict"; +// This file exists only so that we can run the TypeScript compiler in the CI build +// to validate our typings.d.ts file. +exports.__esModule = true; +var ldclient_js_1 = require("ldclient-js"); +var emptyOptions = {}; +var logger = ldclient_js_1.createConsoleLogger("info"); +var allOptions = { + bootstrap: {}, + hash: '', + baseUrl: '', + eventsUrl: '', + streamUrl: '', + streaming: true, + useReport: true, + sendLDHeaders: true, + evaluationReasons: true, + fetchGoals: true, + sendEvents: true, + allAttributesPrivate: true, + privateAttributeNames: ['x'], + allowFrequentDuplicateEvents: true, + sendEventsOnlyForVariation: true, + flushInterval: 1, + samplingInterval: 1, + streamReconnectDelay: 1, + logger: logger +}; +var user = { key: 'user' }; +var client = ldclient_js_1.initialize('env', user, allOptions); From b18507ad710af85e69b54148226b3b6d0a9427ba Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 3 Dec 2018 15:21:57 -0800 Subject: [PATCH 20/20] rm mistakenly checked-in files --- packages/ldclient-electron/test-types.js | 31 ----------------------- packages/ldclient-js-common/test-types.js | 3 --- packages/ldclient-js/test-types.js | 30 ---------------------- 3 files changed, 64 deletions(-) delete mode 100644 packages/ldclient-electron/test-types.js delete mode 100644 packages/ldclient-js-common/test-types.js delete mode 100644 packages/ldclient-js/test-types.js diff --git a/packages/ldclient-electron/test-types.js b/packages/ldclient-electron/test-types.js deleted file mode 100644 index cebd060e..00000000 --- a/packages/ldclient-electron/test-types.js +++ /dev/null @@ -1,31 +0,0 @@ -"use strict"; -// This file exists only so that we can run the TypeScript compiler in the CI build -// to validate our typings.d.ts file. -exports.__esModule = true; -var ldclient_electron_1 = require("ldclient-electron"); -var emptyOptions = {}; -var logger = ldclient_electron_1.createConsoleLogger("info"); -var allOptions = { - bootstrap: {}, - baseUrl: '', - eventsUrl: '', - streamUrl: '', - streaming: true, - useReport: true, - sendLDHeaders: true, - evaluationReasons: true, - sendEvents: true, - allAttributesPrivate: true, - privateAttributeNames: ['x'], - allowFrequentDuplicateEvents: true, - sendEventsOnlyForVariation: true, - flushInterval: 1, - samplingInterval: 1, - streamReconnectDelay: 1, - logger: logger -}; -var user = { key: 'user' }; -var client1 = ldclient_electron_1.initializeMain('env', user, allOptions); -var client2 = ldclient_electron_1.initializeInRenderer('env', allOptions); -var client2WithDefaults = ldclient_electron_1.initializeInRenderer(); -var client3 = ldclient_electron_1.createNodeSdkAdapter(client1); diff --git a/packages/ldclient-js-common/test-types.js b/packages/ldclient-js-common/test-types.js deleted file mode 100644 index fdd666c0..00000000 --- a/packages/ldclient-js-common/test-types.js +++ /dev/null @@ -1,3 +0,0 @@ -"use strict"; -// This file exists only so that we can run the TypeScript compiler in the CI build -// to validate our typings.d.ts file. diff --git a/packages/ldclient-js/test-types.js b/packages/ldclient-js/test-types.js deleted file mode 100644 index f1367896..00000000 --- a/packages/ldclient-js/test-types.js +++ /dev/null @@ -1,30 +0,0 @@ -"use strict"; -// This file exists only so that we can run the TypeScript compiler in the CI build -// to validate our typings.d.ts file. -exports.__esModule = true; -var ldclient_js_1 = require("ldclient-js"); -var emptyOptions = {}; -var logger = ldclient_js_1.createConsoleLogger("info"); -var allOptions = { - bootstrap: {}, - hash: '', - baseUrl: '', - eventsUrl: '', - streamUrl: '', - streaming: true, - useReport: true, - sendLDHeaders: true, - evaluationReasons: true, - fetchGoals: true, - sendEvents: true, - allAttributesPrivate: true, - privateAttributeNames: ['x'], - allowFrequentDuplicateEvents: true, - sendEventsOnlyForVariation: true, - flushInterval: 1, - samplingInterval: 1, - streamReconnectDelay: 1, - logger: logger -}; -var user = { key: 'user' }; -var client = ldclient_js_1.initialize('env', user, allOptions);