Call GPP via code splitting #4447

allisonking · 2023-11-20T19:51:30Z

Closes https://ethyca.atlassian.net/browse/PROD-1395

Description Of Changes

Integrates GPP as a dynamically imported JS package. This means it isn't called until it is needed so isn't included in the initial bundle size. This is also a good proof of concept for how we can implement code splitting in our bundle to split out our UI code from our core initialization code.

Caveats

This works pretty well, but there are some warnings in the Privacy Center logs which say Critical dependency: the request of a dependency is an expression. After some googling, this appears to be a warning that is okay to ignore: vercel/next.js#10633

One way to remove the warning is to force the variable where we cast the GPP bundle parameter to a string but this actually causes the app not to compile. It tries to import typescript files which it can't read, and I haven't figured out why that might be. So I left the warning in, though I can't say I totally understand it.

Code Changes

Set up a new route in privacy center called /gpp-ext
Set up the gpp code as a separate output of rollup which copies to a place the PC expects (in order to host /gpp-ext
New environment variable FIDES_PRIVACY_CENTER__IS_GPP_ENABLED to include the GPP bundle dynamically. In the future, this will be done through a config on the backend, but to get this going, I added the env var. We should remove it down the line, similar to what we did for TCF.
Cypress tests

Steps to Confirm

Run privacy center via turbo dev. This will build the new gpp-ext.js output and you should see a block along the lines of

fides-js:build: src/extensions/gpp.ts → dist/gpp-ext.js...
fides-js:build: ✅ gpp-ext.js gzipped size passed maximum size checks (2.86 KB < 15 KB)
fides-js:build: ┌──────────────────────────────────┐
fides-js:build: │                                  │
fides-js:build: │   Destination: dist/gpp-ext.js   │
fides-js:build: │   Bundle Size:  11.04 KB         │
fides-js:build: │   Minified Size:  10.94 KB       │
fides-js:build: │   Gzipped Size:  2.86 KB         │
fides-js:build: │                                  │
fides-js:build: └──────────────────────────────────┘
fides-js:build: copied:
fides-js:build:   dist/gpp-ext.js → ../privacy-center/public/lib/gpp-ext.js
fides-js:build: created dist/gpp-ext.js in 122ms

Make a note of the gzipped size of the fides-tcf.js bundle. It should be around 40.45 KB
Now visit localhost:3000/fides-js-demo.html?geolocation=fr-id to get the TCF overlay
Open up the console and try to ping the GPP API via __gpp('ping', (data) => {console.log({data})}). You should get an error that the function __gpp is not defined. This is expected since the bundle wasn't included!

Now stop your privacy center and restart it with FIDES_PRIVACY_CENTER__IS_GPP_ENABLED=true
Check the gzipped size of the fides-tcf.js bundle. It should be the same as it was before (40.45 KB).
Now visit localhost:3000/fides-js-demo.html?geolocation=fr-id to get the TCF overlay
Open up the console and try to ping the GPP API via __gpp('ping', (data) => {console.log({data})}). This time you should get a data object back!

In the Network tab, you should also see a new call to gpp-ext.js

Tada, GPP without increasing the initial bundle size!

Pre-Merge Checklist

All CI Pipelines Succeeded
Issue Requirements are Met
Update CHANGELOG.md

vercel · 2023-11-20T19:51:36Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 1, 2023 9:16pm

cypress · 2023-11-20T20:02:42Z

Passing run #5445 ↗︎

0	4	0	0	0

Details:

Merge `d358613` into `6ebfb5d`...
Project: fides	Commit: `bf52fc29c9 ℹ️`
Status: Passed	Duration: 00:33 💡
Started: Dec 1, 2023 9:24 PM	Ended: Dec 1, 2023 9:25 PM

Review all test suite changes for PR #4447 ↗︎

clients/fides-js/src/components/ConsentContent.tsx

allisonking · 2023-11-20T21:39:03Z

clients/fides-js/src/extensions/setup.ts

+
+export const setupExtensions = (options: FidesOptions) => {
+  if (options.gppEnabled) {
+    import(GPP_EXT_PATH).catch((e) => {


this is the dynamic importing step!

Very nice. We'll need to test this on a real web server to see the gotchas though - I'd imagine a relative path like this might be stymied when the script tag is a third-party source?

For example, if we're on example.com and we do this:

<script src="https://ethyca.com/path/to/fides.js" />

Does this know how to import from "https://ethyca.com/path/to/gpp-ext.js", or is it going to try to use "https://example.com/gpp-ext.js"?

hm yeah I think it'll try to use example.com/gpp-ext.js. how do we want to treat this? should we try to force it to look relative, or allow the whole path to be configurable?

Realistically, we'll want to make it configurable, and then test out what works well in our staging/etc

clients/fides-js/src/lib/consent-types.ts

NevilleS

This is super promising! I have some nitpicky comments about filenames (mostly!) but my main question is: will this work once it's deployed? 😄

clients/fides-js/rollup.config.mjs

NevilleS · 2023-11-22T18:29:27Z

clients/fides-js/src/extensions/setup.ts

+
+export const setupExtensions = (options: FidesOptions) => {
+  if (options.gppEnabled) {
+    import(GPP_EXT_PATH).catch((e) => {


Very nice. We'll need to test this on a real web server to see the gotchas though - I'd imagine a relative path like this might be stymied when the script tag is a third-party source?

For example, if we're on example.com and we do this:

<script src="https://ethyca.com/path/to/fides.js" />

Does this know how to import from "https://ethyca.com/path/to/gpp-ext.js", or is it going to try to use "https://example.com/gpp-ext.js"?

clients/fides-js/src/lib/consent-types.ts

clients/fides-js/src/lib/initialize.ts

clients/privacy-center/next.config.js

NevilleS · 2023-11-22T19:22:55Z

clients/privacy-center/pages/api/gpp-js.ts

+export default async function handler(
+  req: NextApiRequest,
+  res: NextApiResponse
+) {
+  const gppJsFile = "public/lib/gpp-ext.js";


FWIW: I feel like this an API route here is more complicated than we'd want here, just because next.js will happily serve static files... but at the same time, it's fairly painful to force cache headers on them...

In other words, a potentially simpler way to solve this is:

copy that fides-gpp-ext.js file into public/ instead of public/lib

use the default hosting of static files from next

add the required headers in the next.config.js or similar

oh good point, that makes things a lot simpler!

hm actually there seem to be some open issues about adding cache headers in the public folder: vercel/next.js#22319

will look into some of the workarounds on monday...

https://stackoverflow.com/questions/62077589/setting-cache-control-header-for-static-file-serving-on-nextjs-default-server

since I couldn't change the cache control header via the public/ folder, I kept this as an endpoint that just serves the file (+ a cache header). I did make the changes to the filenames so the rollup file is easier to follow now though. if you have ideas for how to make caching work on public/ then we can come back to this?

Yup, no problem!

allisonking · 2023-11-27T15:57:16Z

I'm running into some issues merging main because of this bit that was just introduced in a separate PR:

fides/clients/fides-js/src/lib/tcf/events.ts

Lines 19 to 22 in 1c500ab

    
           if (!window.Fides.options.fidesTcfGdprApplies) { 
        
             // A `null` TC string means gdpr does not apply 
        
             return null; 
        
           }

This PR also has GPP calling the same function, however I'm unclear about how to get the GPP extension file to also have access to window.Fides. I know it has something to do with the rollup output here

fides/clients/fides-js/rollup.config.mjs

Lines 105 to 113 in fae7acf

    
           output: [ 
        
             { 
        
               // Intended for browser <script> tag - defines `Fides` global. Also supports UMD loaders. 
        
               file: `${outputFileName}.js`, 
        
               name: isExtension ? undefined : "Fides", 
        
               format: isExtension ? undefined : "umd", 
        
               sourcemap: IS_DEV, 
        
             }, 
        
           ],

But I'm not sure the right way to tell the GPP extension to also use the same window.Fides from one of the base fides.js files

allisonking · 2023-11-27T20:11:13Z

I'm running into some issues merging main because of this bit that was just introduced in a separate PR:

Figured it out! I just needed to make sure my GPP ext also had the same window declaration in order to access the Fides object.

allisonking · 2023-12-01T19:09:42Z

clients/fides-js/src/lib/initialize.ts

+  // Call extensions
+  // DEFER(PROD#1439): This is likely too late for the GPP stub.
+  // We should move stub code out to the base package and call it right away instead.
+  await setupExtensions(options);


I just changed this to await because otherwise FidesInitialized might be called before the extension is ready to listen for it. I'm not sure if this is okay though in terms of initialization times—the first FidesInitialized from an existing cookie will already have fired, so this is counting on the second one to fire. It would happen after the UI initializes, which might be far enough away from initial load times?

For the stub initialization, it'd definitely be too late.

NevilleS

Overall: I'd say ship it! I think you should add a config option here though to experiment with the import path.

NevilleS · 2023-12-01T19:26:50Z

clients/fides-js/src/lib/initialize.ts

+  // Call extensions
+  // DEFER(PROD#1439): This is likely too late for the GPP stub.
+  // We should move stub code out to the base package and call it right away instead.
+  await setupExtensions(options);


For the stub initialization, it'd definitely be too late.

NevilleS · 2023-12-01T19:28:48Z

clients/fides-js/src/extensions/setup.ts

+
+export const setupExtensions = (options: FidesOptions) => {
+  if (options.gppEnabled) {
+    import(GPP_EXT_PATH).catch((e) => {


Realistically, we'll want to make it configurable, and then test out what works well in our staging/etc

NevilleS · 2023-12-01T19:29:06Z

clients/privacy-center/pages/api/gpp-js.ts

+export default async function handler(
+  req: NextApiRequest,
+  res: NextApiResponse
+) {
+  const gppJsFile = "public/lib/gpp-ext.js";


Yup, no problem!

NevilleS · 2023-12-01T19:30:46Z

clients/fides-js/src/lib/extensions.ts

+ * The path where the GPP extension bundle is located,
+ * likely served by the privacy center
+ */
+const GPP_EXT_PATH = "/fides-ext-gpp.js";


I do this we should make this configurable, especially as a runtime override option. That'd give a lot of flexibility for testing this out in different environments

Good idea! 17b8fa1

Running FIDES_PRIVACY_CENTER__IS_GPP_ENABLED=true FIDES_PRIVACY_CENTER__GPP_EXTENSION_PATH="/test.js" turbo dev will try to import the GPP path from /test.js and you'll get an error in the console. Running without the env var will default to /fides-ext-gpp.js and things should work!

allisonking added 3 commits November 20, 2023 13:48

Early poc for dynamic importing

344bd81

Output gpp-ext.js from rollup

8432deb

Add a gpp enabled environment variable

fc9b4b7

allisonking added 2 commits November 20, 2023 15:35

Resolve circular dependencies

a394a05

Move gpp call to base initialize logic

b860050

allisonking commented Nov 20, 2023

View reviewed changes

clients/fides-js/src/components/ConsentContent.tsx Show resolved Hide resolved

allisonking commented Nov 20, 2023

View reviewed changes

clients/fides-js/src/lib/consent-types.ts Show resolved Hide resolved

allisonking added 4 commits November 20, 2023 16:45

Update gpp openapi

336c0fe

Update comment

c0a0766

Update changelog

ffb941f

Merge branch 'main' into aking/prod-1395/code-splitting-poc

79ad47f

allisonking marked this pull request as ready for review November 21, 2023 16:12

allisonking requested review from NevilleS and eastandwestwind November 21, 2023 16:12

Cleanup changelog

fae7acf

NevilleS requested changes Nov 22, 2023

View reviewed changes

allisonking added 4 commits November 27, 2023 12:13

Rework GPP paths and names to simplify rollup

95cd2ed

Import gpp ext as a module

eb02370

Merge branch 'main' into aking/prod-1395/code-splitting-poc

1e1a800

Fix type declaration

f2e2de9

allisonking added 3 commits November 30, 2023 12:46

Merge branch 'main' into aking/prod-1395/code-splitting-poc

cf8c45f

Tweak some event orders

825de1e

Merge branch 'main' into aking/prod-1395/code-splitting-poc

9abf70f

vercel bot deployed to Preview December 1, 2023 17:46 View deployment

Await extension setup

bd9663a

allisonking commented Dec 1, 2023

View reviewed changes

NevilleS approved these changes Dec 1, 2023

View reviewed changes

allisonking added 3 commits December 1, 2023 14:53

Clean up console.logs

d01619d

Allow configurable GPP extension path

17b8fa1

Merge branch 'main' into aking/prod-1395/code-splitting-poc

d358613

vercel bot deployed to Preview December 1, 2023 21:16 View deployment

allisonking merged commit fc490d6 into main Dec 1, 2023
13 checks passed

allisonking deleted the aking/prod-1395/code-splitting-poc branch December 1, 2023 22:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Call GPP via code splitting #4447

Call GPP via code splitting #4447

allisonking commented Nov 20, 2023 •

edited

Loading

vercel bot commented Nov 20, 2023 •

edited

Loading

cypress bot commented Nov 20, 2023 •

edited

Loading

allisonking Nov 20, 2023

NevilleS Nov 22, 2023

allisonking Nov 22, 2023

NevilleS Dec 1, 2023

NevilleS left a comment

NevilleS Nov 22, 2023

NevilleS Nov 22, 2023

allisonking Nov 22, 2023

allisonking Nov 22, 2023

allisonking Nov 22, 2023

allisonking Nov 27, 2023

NevilleS Dec 1, 2023

allisonking commented Nov 27, 2023

allisonking commented Nov 27, 2023

allisonking Dec 1, 2023

NevilleS Dec 1, 2023

NevilleS left a comment

NevilleS Dec 1, 2023

NevilleS Dec 1, 2023

NevilleS Dec 1, 2023

NevilleS Dec 1, 2023

allisonking Dec 1, 2023

Call GPP via code splitting #4447

Call GPP via code splitting #4447

Conversation

allisonking commented Nov 20, 2023 • edited Loading

Description Of Changes

Caveats

Code Changes

Steps to Confirm

Pre-Merge Checklist

vercel bot commented Nov 20, 2023 • edited Loading

cypress bot commented Nov 20, 2023 • edited Loading

Passing run #5445 ↗︎

Review all test suite changes for PR #4447 ↗︎

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NevilleS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allisonking commented Nov 27, 2023

allisonking commented Nov 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NevilleS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allisonking commented Nov 20, 2023 •

edited

Loading

vercel bot commented Nov 20, 2023 •

edited

Loading

cypress bot commented Nov 20, 2023 •

edited

Loading