Skip to content
This repository was archived by the owner on Sep 3, 2022. It is now read-only.

Replace @ndhoule/defaults with ES6 spread syntax merging #185

Merged
merged 7 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 4.1.5 / 2020-09-17

- Replace @ndhoule/defaults with merging via ES6 spread syntax

# 4.1.4 / 2020-09-16

- Replace `@ndhoule/includes` with `lodash.includes`
Expand Down
60 changes: 47 additions & 13 deletions lib/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {
InitOptions,
SegmentAnalytics,
SegmentOpts,
SegmentIntegration
SegmentIntegration,
PageDefaults, Message
} from './types';

import cloneDeep from 'lodash.clonedeep'
Expand Down Expand Up @@ -323,8 +324,13 @@ Analytics.prototype.identify = function(
});

// Add the initialize integrations so the server-side ones can be disabled too
// NOTE: We need to merge integrations, not override them with assign
// since it is possible to change the initialized integrations at runtime.
if (this.options.integrations) {
defaults(msg.integrations, this.options.integrations);
msg.integrations = {
...this.options.integrations,
...msg.integrations
}
}

this._invoke('identify', new Identify(msg));
Expand Down Expand Up @@ -379,8 +385,13 @@ Analytics.prototype.group = function(
});

// Add the initialize integrations so the server-side ones can be disabled too
// NOTE: We need to merge integrations, not override them with assign
// since it is possible to change the initialized integrations at runtime.
if (this.options.integrations) {
defaults(msg.integrations, this.options.integrations);
msg.integrations = {
...this.options.integrations,
...msg.integrations
}
}

this._invoke('group', new Group(msg));
Expand Down Expand Up @@ -444,10 +455,12 @@ Analytics.prototype.track = function(
}

// Add the initialize integrations so the server-side ones can be disabled too
defaults(
msg.integrations,
this._mergeInitializeAndPlanIntegrations(planIntegrationOptions)
);
// NOTE: We need to merge integrations, not override them with assign
// since it is possible to change the initialized integrations at runtime.
msg.integrations = {
...this._mergeInitializeAndPlanIntegrations(planIntegrationOptions),
...msg.integrations
}

this._invoke('track', new Track(msg));

Expand Down Expand Up @@ -600,8 +613,15 @@ Analytics.prototype.page = function(

// Ensure properties has baseline spec properties.
// TODO: Eventually move these entirely to `options.context.page`
const defs = pageDefaults();
defaults(properties, defs);
// FIXME: This is purposely not overriding `defs`. There was a bug in the logic implemented by `@ndhoule/defaults`.
// This bug made it so we only would overwrite values in `defs` that were set to `undefined`.
// In some cases, though, pageDefaults will return defaults with values set to "" (such as `window.location.search` defaulting to "").
// The decision to not fix this bus was made to preserve backwards compatibility.
const defs = pageDefaults()
properties = {
...properties,
...defs
}

// Mirror user overrides to `options.context.page` (but exclude custom properties)
// (Any page defaults get applied in `this.normalize` for consistency.)
Expand All @@ -621,8 +641,13 @@ Analytics.prototype.page = function(
});

// Add the initialize integrations so the server-side ones can be disabled too
// NOTE: We need to merge integrations, not override them with assign
// since it is possible to change the initialized integrations at runtime.
if (this.options.integrations) {
defaults(msg.integrations, this.options.integrations);
msg.integrations = {
...this.options.integrations,
...msg.integrations
}
}

this._invoke('page', new Page(msg));
Expand Down Expand Up @@ -674,8 +699,13 @@ Analytics.prototype.alias = function(
});

// Add the initialize integrations so the server-side ones can be disabled too
// NOTE: We need to merge integrations, not override them with assign
// since it is possible to change the initialized integrations at runtime.
if (this.options.integrations) {
defaults(msg.integrations, this.options.integrations);
msg.integrations = {
...this.options.integrations,
...msg.integrations
}
}

this._invoke('alias', new Alias(msg));
Expand Down Expand Up @@ -967,15 +997,19 @@ Analytics.prototype._parseQuery = function(query: string): SegmentAnalytics {
*/

Analytics.prototype.normalize = function(msg: {
context: { page };
options: { [key: string]: unknown }
context: { page: Partial<PageDefaults> };
anonymousId: string;
}): object {
msg = normalize(msg, Object.keys(this._integrations));
if (msg.anonymousId) user.anonymousId(msg.anonymousId);
msg.anonymousId = user.anonymousId();

// Ensure all outgoing requests include page data in their contexts.
msg.context.page = defaults(msg.context.page || {}, pageDefaults());
msg.context.page = {
...pageDefaults(),
...msg.context.page
};

return msg;
};
Expand Down
19 changes: 12 additions & 7 deletions lib/cookie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import cloneDeep from 'lodash.clonedeep'
var bindAll = require('bind-all');
var cookie = require('@segment/cookie');
var debug = require('debug')('analytics.js:cookie');
var defaults = require('@ndhoule/defaults');
var topDomain = require('@segment/top-domain');

const MAX_AGE_ONE_YEAR = 31536000000

/**
* Initialize a new `Cookie` with `options`.
*
Expand All @@ -32,16 +33,20 @@ Cookie.prototype.options = function(options?: CookieOptions) {

options = options || {};

var domain = '.' + topDomain(window.location.href);
let domain = '.' + topDomain(window.location.href);
if (domain === '.') domain = null;

this._options = defaults(options, {
// default to a year
maxage: 31536000000,
path: '/',
const defaults: CookieOptions = {
maxage: MAX_AGE_ONE_YEAR,
domain: domain,
path: '/',
sameSite: 'Lax'
});
}

this._options = {
...defaults,
...options
};

// http://curl.haxx.se/rfc/cookie_spec.html
// https://publicsuffix.org/list/effective_tld_names.dat
Expand Down
6 changes: 4 additions & 2 deletions lib/entity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import assignIn from 'lodash.assignin'

var cookie = require('./cookie');
var debug = require('debug')('analytics:entity');
var defaults = require('@ndhoule/defaults');
var memory = require('./memory');
var store = require('./store');
var isodateTraverse = require('@segment/isodate-traverse');
Expand Down Expand Up @@ -74,7 +73,10 @@ Entity.prototype.storage = function() {

Entity.prototype.options = function(options?: InitOptions) {
if (arguments.length === 0) return this._options;
this._options = defaults(options || {}, this.defaults || {});
this._options = {
...this.defaults,
...options
}
};

/**
Expand Down
8 changes: 6 additions & 2 deletions lib/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import includes from 'lodash.includes'
*/

var debug = require('debug')('analytics.js:normalize');
var defaults = require('@ndhoule/defaults');
var type = require('component-type');
var uuid = require('uuid/v4');
var md5 = require('spark-md5').hash;


/**
* HOP.
*/
Expand Down Expand Up @@ -88,7 +88,11 @@ function normalize(msg: Message, list: Array<any>): NormalizedMessage {
delete msg.options;
ret.integrations = integrations;
ret.context = context;
ret = defaults(ret, msg);
ret = {
...msg,
...ret
}

debug('->', ret);
return ret;

Expand Down
10 changes: 7 additions & 3 deletions lib/store.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
'use strict';

import { StoreOptions } from './types';

/*
* Module dependencies.
*/

var bindAll = require('bind-all');
var defaults = require('@ndhoule/defaults');
var store = require('@segment/store');

/**
Expand All @@ -22,11 +23,14 @@ function Store(options?: { enabled: boolean }) {
* Set the `options` for the store.
*/

Store.prototype.options = function(options?: { enabled?: boolean }) {
Store.prototype.options = function(options?: StoreOptions) {
if (arguments.length === 0) return this._options;

options = options || {};
defaults(options, { enabled: true });
options = {
enabled: true,
...options
};

this.enabled = options.enabled && store.enabled;
this._options = options;
Expand Down
3 changes: 2 additions & 1 deletion lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface CookieOptions {
domain?: string;
path?: string;
secure?: boolean;
sameSite?: string
}

export interface MetricsOptions {
Expand All @@ -46,7 +47,7 @@ export interface MetricsOptions {
maxQueueSize?: number;
}

interface StoreOptions {
export interface StoreOptions {
enabled?: boolean;
}

Expand Down
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@segment/analytics.js-core",
"author": "Segment <[email protected]>",
"version": "4.1.4",
"version": "4.1.5",
"description": "The hassle-free way to integrate analytics into any web application.",
"types": "lib/index.d.ts",
"keywords": [
Expand Down Expand Up @@ -30,7 +30,6 @@
},
"homepage": "https://github.com/segmentio/analytics.js-core#readme",
"dependencies": {
"@ndhoule/defaults": "^2.0.1",
"@segment/canonical": "^1.0.0",
"@segment/cookie": "^1.1.5",
"@segment/is-meta": "^1.0.0",
Expand Down
40 changes: 40 additions & 0 deletions test/analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var pageDefaults = require('../build/pageDefaults');
var sinon = require('sinon');
var tick = require('next-tick');
var trigger = require('compat-trigger-event');

var Identify = Facade.Identify;
var cookie = Analytics.cookie;
var group = analytics.group();
Expand Down Expand Up @@ -828,6 +829,22 @@ describe('Analytics', function() {
});
});

it('should should not overwrite context.page values due to an existing bug', function() {
analytics.page({ prop: true, context: { page: { search: "lol" } } }, { context: { page: { search: "" } } });
var page = analytics._invoke.args[0][1];
// FIXME: This assert should fail once the bug is fixed
assert.notDeepEqual(page.context(), {
page: {
...defaults,
search: "lol",
},
});
// FIXME: This assert should fail once the bug is fixed
assert.deepEqual(page.context(), {
page: defaults,
});
});

it('should emit page', function(done) {
analytics.once('page', function(category, name, props, opts) {
assert(category === 'category');
Expand Down Expand Up @@ -1608,6 +1625,29 @@ describe('Analytics', function() {
assert.deepEqual(track.context(), { page: contextPage });
});

it('should support overwriting context.page fields', function() {
analytics.track(
'event',
{},
{
context: {
page: {
title: 'lol'
}
}
}
);

var track = analytics._invoke.args[0][1];
assert.deepEqual(
track.context().page,
{
...contextPage,
title: 'lol'
}
);
});

it('should accept context.traits', function() {
analytics.track('event', { prop: 1 }, { traits: { trait: true } });
var track = analytics._invoke.args[0][1];
Expand Down
9 changes: 9 additions & 0 deletions test/cookie.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ describe('cookie', function() {
assert(cookie.options().maxage === 31536000000);
});

it('should have default options', function() {
cookie.options({ domain: '' });

assert(cookie.options().maxage === 31536000000);
assert(cookie.options().path === '/');
assert(cookie.options().domain === '');
assert(cookie.options().sameSite === 'Lax');
});

it('should set the domain correctly', function() {
cookie.options({ domain: '' });
assert(cookie.options().domain === '');
Expand Down
12 changes: 12 additions & 0 deletions test/normalize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,18 @@ describe('normalize', function() {
}
});
});

it('should merge with defaults', function() {
opts.context = { foo: 5 };
var out = normalize(msg, list);
assert.deepEqual(out.integrations, {});
assert.deepEqual(out.context, { foo: 5 });

msg.options = { integrations: { Segment: true }, context: { foo: 6 } };
out = normalize(msg, list);
assert.deepEqual(out.integrations, { Segment: true });
assert.deepEqual(out.context, { foo: 6 });
});
});

describe('integrations', function() {
Expand Down
6 changes: 6 additions & 0 deletions test/store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,11 @@ describe('store', function() {
assert(store.options().enabled === false);
assert(store.enabled === false);
});

it('should have default options', function() {
store.options({});

assert(store.options().enabled);
});
});
});
Loading