Skip to content

Commit

Permalink
Build local copy of Digital Analytics Program (#11097)
Browse files Browse the repository at this point in the history
* Build local copy of Digital Analytics Program

changelog: Internal, Performance, Optimize loading of Digital Analytics Program script

* Remove CSP exceptions for DAP CDN

* Download and patch DAP as postinstall

* Consistently return string value from UriService#add_params

* Add spec to load DAP script

* Update DAP script test to check syntax error on load

DAP script has too many side effects and manipulates DOM in a way that's hard to clean up. Absent some other way to create an isolated DOM, at least check that the script can be evaluated without syntax errors.

* Add DAP script to ESLint ignore

* Add test for url_params behavior

* Better coverage for combination url_params and attributes

* Treat url_params as distinct property

See: #11097 (comment)

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Use conventional capitalization

See: https://github.com/18F/identity-idp/blob/main/docs/frontend.md#naming-conventions

* Separate download and patch tasks to cache-bust by SHA

* Ignore DAP script in TypeScript checks

* Use more Makefile wizardry to generate compiled DAP

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Run analytics postinstall using Yarn cwd

In production environments, `yarn install --production` won't make workspaces available, so trying to use `yarn workspace ...` will fail. This achieves the same effect without relying on the workspace being "installed"

* Try avoiding default NPM lifecycle script names

* Test using full install in cwd package

* Try incorporating analytics tasks in build:js

In CI Dockerfile, app files aren't available at  time of install. This also aligns closer to how browsers.json is generated

* Combine make tasks in build:js

* Force analytics task in top-level Makefile

Defer to sub-process to decide what needs to be done

* Split install and build tasks for analytics

Avoid dependency on package code existing during install step

* Copy Makefile earlier in Docker images

* Try complete build if folder available

* Revert to original postinstall approach

---------

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
  • Loading branch information
aduth and zachmargolis authored Aug 20, 2024
1 parent 5bb3cc9 commit 85df0cf
Show file tree
Hide file tree
Showing 17 changed files with 130 additions and 13 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ public
vendor
coverage
doc
app/javascript/packages/analytics/digital-analytics-program*.js
1 change: 0 additions & 1 deletion app/controllers/users/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ def override_csp_for_google_analytics
policy = current_content_security_policy
policy.script_src(
*policy.script_src,
'dap.digitalgov.gov',
'www.google-analytics.com',
'www.googletagmanager.com',
)
Expand Down
8 changes: 4 additions & 4 deletions app/helpers/script_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

# rubocop:disable Rails/HelperInstanceVariable
module ScriptHelper
def javascript_packs_tag_once(*names, **attributes)
@scripts = @scripts.to_h.merge(names.index_with(attributes))
def javascript_packs_tag_once(*names, url_params: nil, **attributes)
@scripts = @scripts.to_h.merge(names.index_with([url_params, attributes]))
nil
end

Expand All @@ -14,10 +14,10 @@ def render_javascript_pack_once_tags(...)
javascript_packs_tag_once(...)
return if @scripts.blank?
concat javascript_assets_tag
@scripts.each do |name, attributes|
@scripts.each do |name, (url_params, attributes)|
asset_sources.get_sources(name).each do |source|
concat javascript_include_tag(
source,
UriService.add_params(source, url_params),
**attributes,
crossorigin: local_crossorigin_sources? ? true : nil,
integrity: asset_sources.get_integrity(source),
Expand Down
1 change: 1 addition & 0 deletions app/javascript/packages/analytics/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
digital-analytics-program*.js
12 changes: 12 additions & 0 deletions app/javascript/packages/analytics/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
DAP_SHA ?= 7c14bb3

digital-analytics-program.js: digital-analytics-program-$(DAP_SHA).js digital-analytics-program.patch
patch -p1 $^ --output $@

digital-analytics-program-$(DAP_SHA).js:
curl https://raw.githubusercontent.com/digital-analytics-program/gov-wide-code/$(DAP_SHA)/Universal-Federated-Analytics.js --silent --output $@

clean:
rm digital-analytics-program-$(DAP_SHA).js digital-analytics-program.js

.PHONY: clean
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
73a74
> GA4Object.async = true;
785d785
< var piiRegex = [];
900c900
< piiRegex.forEach(function (pii) {
---
> window.piiRegex.forEach(function (pii) {
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Worker } from 'node:worker_threads';
import { join } from 'node:path';
import { pathToFileURL } from 'node:url';

describe('digital analytics program', () => {
it('parses without syntax error', async () => {
// Future: Replace with Promise.withResolvers once supported
// See: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers
let resolve;
const promise = new Promise((_resolve) => {
resolve = _resolve;
});

// Reference: https://github.com/nodejs/node/issues/30682
const toDataURL = (source: string) =>
new URL(`data:text/javascript,${encodeURIComponent(source)}`);
const url = pathToFileURL(join(__dirname, './digital-analytics-program.js'));
const code = `await import(${JSON.stringify(url)});`;
new Worker(toDataURL(code)).on('error', (error) => {
expect(error).not.to.be.instanceOf(SyntaxError);
resolve();
});

await promise;
});
});
11 changes: 10 additions & 1 deletion app/javascript/packages/analytics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,16 @@
"name": "@18f/identity-analytics",
"private": true,
"version": "1.0.0",
"scripts": {
"postinstall": "make digital-analytics-program.js"
},
"exports": {
".": "./index.ts",
"./click-observer-element": "./click-observer-element.ts",
"./digital-analytics-program": "./digital-analytics-program.js"
},
"sideEffects": [
"./click-observer-element.ts"
"./click-observer-element.ts",
"./digital-analytics-program.js"
]
}
1 change: 1 addition & 0 deletions app/javascript/packs/digital-analytics-program.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '@18f/identity-analytics/digital-analytics-program';
5 changes: 3 additions & 2 deletions app/services/uri_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ def self.params(original_uri)

# @param [#to_s] original_uri
# @param [Hash, nil] params_to_add
# @return [URI, nil]
# @return [String, nil]
def self.add_params(original_uri, params_to_add)
return if original_uri.blank?
return original_uri.to_s if params_to_add.blank?

URI(original_uri).tap do |uri|
query = params(uri).merge(params_to_add || {})
query = params(uri).merge(params_to_add)
uri.query = query.empty? ? nil : query.to_query
end.to_s
rescue URI::BadURIError, URI::InvalidURIError
Expand Down
3 changes: 2 additions & 1 deletion app/views/devise/sessions/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@
<% if IdentityConfig.store.participate_in_dap %>
<!-- <%= t('notices.dap_participation') %> -->
<%= javascript_packs_tag_once(
'https://dap.digitalgov.gov/Universal-Federated-Analytics-Min.js?agency=GSA&subagency=TTS',
'digital-analytics-program',
url_params: { agency: 'GSA', subagency: 'TTS' },
defer: true,
id: '_fed_an_ua_tag',
preload_links_header: false,
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"app/javascript/packages/*"
],
"scripts": {
"postinstall": "yarn workspace @18f/identity-analytics run postinstall",
"typecheck": "tsc",
"lint": "eslint . --ext .js,.jsx,.ts,.tsx",
"lint:css": "stylelint 'app/assets/stylesheets/**/*.scss' 'app/javascript/**/*.scss' 'app/components/*.scss'",
Expand Down
32 changes: 32 additions & 0 deletions spec/helpers/script_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,38 @@
end
end

context 'with url parameters' do
before do
javascript_packs_tag_once(
'digital-analytics-program',
url_params: { agency: 'gsa' },
async: true,
)
allow(Rails.application.config.asset_sources).to receive(:get_sources).
with('digital-analytics-program').and_return(['/digital-analytics-program.js'])
allow(Rails.application.config.asset_sources).to receive(:get_assets).
with('application', 'document-capture', 'digital-analytics-program').
and_return([])
end

it 'includes url parameters in script url for the pack' do
output = render_javascript_pack_once_tags

expect(output).to have_css(
"script[src^='/digital-analytics-program.js?agency=gsa'][async]:not([url_params])",
count: 1,
visible: :all,
)

# URL parameters should not be added to other scripts
expect(output).to have_css(
"script[src^='/application.js']",
count: 1,
visible: :all,
)
end
end

context 'local development crossorigin sources' do
let(:webpack_port) { '3035' }

Expand Down
2 changes: 0 additions & 2 deletions spec/requests/csp_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@
content_security_policy = parse_content_security_policy

# See: https://github.com/digital-analytics-program/gov-wide-code#content-security-policy
expect(content_security_policy['script-src']).to include('dap.digitalgov.gov')
expect(content_security_policy['script-src']).to include('www.google-analytics.com')
expect(content_security_policy['script-src']).to include('www.googletagmanager.com')
expect(content_security_policy['connect-src']).to include('www.google-analytics.com')
Expand All @@ -243,7 +242,6 @@
content_security_policy = parse_content_security_policy

# See: https://github.com/digital-analytics-program/gov-wide-code#content-security-policy
expect(content_security_policy['script-src']).not_to include('dap.digitalgov.gov')
expect(content_security_policy['script-src']).not_to include('www.google-analytics.com')
expect(content_security_policy['script-src']).not_to include('www.googletagmanager.com')
expect(content_security_policy['connect-src']).not_to include('www.google-analytics.com')
Expand Down
22 changes: 22 additions & 0 deletions spec/services/uri_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,27 @@

expect(uri).to eq('https://example.com/foo/bar/?query=value#fragment')
end

context 'with a uri object' do
let(:params_to_add) {}
let(:original_uri) { URI('https://example.com/foo/bar/') }
subject(:uri) { UriService.add_params(original_uri, params_to_add) }

context 'without parameters to add' do
let(:params_to_add) { nil }

it 'returns the original uri as a string' do
expect(uri).to eq('https://example.com/foo/bar/')
end
end

context 'with parameters to add' do
let(:params_to_add) { { query: 'value' } }

it 'returns a string of the original uri with parameters added' do
expect(uri).to eq('https://example.com/foo/bar/?query=value')
end
end
end
end
end
3 changes: 2 additions & 1 deletion spec/views/devise/sessions/new.html.erb_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@
it 'renders DAP analytics' do
allow(view).to receive(:javascript_packs_tag_once)
expect(view).to receive(:javascript_packs_tag_once).with(
a_string_matching('https://dap.digitalgov.gov/'),
'digital-analytics-program',
url_params: { agency: 'GSA', subagency: 'TTS' },
defer: true,
preload_links_header: false,
id: '_fed_an_ua_tag',
Expand Down
6 changes: 5 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,9 @@
"./*.js",
"scripts"
],
"exclude": ["**/fixtures", "spec/**/*.spec.js"]
"exclude": [
"**/fixtures",
"spec/**/*.spec.js",
"app/javascript/packages/analytics/digital-analytics-program*.js"
]
}

0 comments on commit 85df0cf

Please sign in to comment.