From f5473d3296e8e22abec3203e72940b229db9af00 Mon Sep 17 00:00:00 2001 From: Stefan Lau Date: Fri, 3 Nov 2017 17:48:50 +0100 Subject: [PATCH 1/2] Fix invalid javascript being generated --- package.json | 1 + src/build_parts.js | 2 +- test/src/build_parts.spec.js | 29 ++++++++++++++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index b5a3c62..a5d22bb 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ }, "homepage": "https://github.com/holidaycheck/react-google-tag-manager#readme", "devDependencies": { + "acorn": "5.2.1", "babel-cli": "6.18.0", "babel-preset-es2015": "6.18.0", "babel-preset-react": "6.16.0", diff --git a/src/build_parts.js b/src/build_parts.js index 9baa114..03a130f 100644 --- a/src/build_parts.js +++ b/src/build_parts.js @@ -16,7 +16,7 @@ function buildParts({ id, dataLayerName = 'dataLayer', additionalEvents = {}, sc w[l].push({'gtm.start': new Date().getTime(),event:'gtm.js', ${convertToKeyValueString(additionalEvents)}}); var f=d.getElementsByTagName(s)[0],j=d.createElement(s),dl=l!='dataLayer'?'&l='+l:''; j.async=true;j.src='${scheme}//www.googletagmanager.com/gtm.js?id='+i+dl - +${previewVariables ? previewVariables : ''}; + ${previewVariables ? `+"${previewVariables}"` : ''}; f.parentNode.insertBefore(j,f); })(window,document,'script','${dataLayerName}','${id}');`; diff --git a/test/src/build_parts.spec.js b/test/src/build_parts.spec.js index dafd428..e403323 100644 --- a/test/src/build_parts.spec.js +++ b/test/src/build_parts.spec.js @@ -1,6 +1,23 @@ -import { expect } from 'chai'; +import { expect, Assertion } from 'chai'; +import { parse } from 'acorn'; import buildParts from '../../src/build_parts'; +Assertion.addMethod('javascript', function () { + let isValidJS = false; + try { + parse(this._obj, { ecmaVersion: 5 }); + isValidJS = true; + } catch (e) { + isValidJS = false; + } + + this.assert( + isValidJS, + 'expected #{this} to be valid JavaScript', + 'expected #{this} to not be valid JavaScript' + ); +}); + let onlyIdArgs; let parts; @@ -18,6 +35,10 @@ describe('The function buildParts', () => { expect(parts.script).to.have.entriesCount('GTM-asd123', 1); }); + it('should build valid javascript', () => { + expect(parts.script).to.be.javascript(); + }); + it('should through an exception when no id is provided', () => { expect(() => buildParts()).to.throw(Error); }); @@ -31,9 +52,11 @@ describe('The function buildParts', () => { it('should consume a `previewVariables`', () => { const dataLayerArgs = Object.assign(onlyIdArgs, { previewVariables: '>m_auth=EXAMPLE>m_preview=env-14>m_cookies_win=x' }); + const script = buildParts(dataLayerArgs).script; - expect(buildParts(dataLayerArgs).script).to.have - .entriesCount('>m_auth=EXAMPLE>m_preview=env-14>m_cookies_win=x', 1); + expect(script).to.be.javascript(); + expect(script).to.have + .entriesCount('+">m_auth=EXAMPLE>m_preview=env-14>m_cookies_win=x"', 1); }); it('should have a `dataLayerName` default', () => { From d60d5f9c6fbbabe58d0b5efb5a960adc3bd2e284 Mon Sep 17 00:00:00 2001 From: Stefan Lau Date: Fri, 3 Nov 2017 17:51:39 +0100 Subject: [PATCH 2/2] Test that valid javascript is generated in every test --- test/src/build_parts.spec.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/src/build_parts.spec.js b/test/src/build_parts.spec.js index e403323..61f564c 100644 --- a/test/src/build_parts.spec.js +++ b/test/src/build_parts.spec.js @@ -45,8 +45,10 @@ describe('The function buildParts', () => { it('should consume a `dataLayerName`', () => { const dataLayerArgs = Object.assign(onlyIdArgs, { dataLayerName: 'MyFooBarLayer' }); + const script = buildParts(dataLayerArgs).script; - expect(buildParts(dataLayerArgs).script).to.have.entriesCount('MyFooBarLayer', 1); + expect(script).to.be.javascript(); + expect(script).to.have.entriesCount('MyFooBarLayer', 1); }); it('should consume a `previewVariables`', () => { @@ -65,8 +67,10 @@ describe('The function buildParts', () => { it('should use a provided `scheme` option', () => { const schemaWithIdArgs = Object.assign(onlyIdArgs, { scheme: 'https:' }); + const script = buildParts(schemaWithIdArgs).script; - expect(buildParts(schemaWithIdArgs).script).to.have.entriesCount('https:', 1); + expect(script).to.be.javascript(); + expect(script).to.have.entriesCount('https:', 1); expect(buildParts(schemaWithIdArgs).iframe).to.have.entriesCount('https:', 1); }); @@ -82,10 +86,12 @@ describe('The function buildParts', () => { clientTimestamp: 1465848238816 }; const addtionalEventsArgs = Object.assign(onlyIdArgs, { additionalEvents }); + const script = buildParts(addtionalEventsArgs).script; - expect(buildParts(addtionalEventsArgs).script).to.have.entriesCount('"platform":"react-stack"', 1); - expect(buildParts(addtionalEventsArgs).script).to.have.entriesCount('"forceMobile":false', 1); - expect(buildParts(addtionalEventsArgs).script).to.have.entriesCount('"clientTimestamp":1465848238816', 1); + expect(script).to.be.javascript(); + expect(script).to.have.entriesCount('"platform":"react-stack"', 1); + expect(script).to.have.entriesCount('"forceMobile":false', 1); + expect(script).to.have.entriesCount('"clientTimestamp":1465848238816', 1); }); it('should return an object with a property `iframe`', () => {