-
Notifications
You must be signed in to change notification settings - Fork 4
Switch to injecting in-app SDK from CDN #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Removed NPM dependency for gist web - Added update_cdp script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR switches from using the NPM package customerio-gist-web
to dynamically loading the Gist library from a CDN. This change allows for remote injection of the in-app SDK without requiring package updates.
Key changes:
- Removes NPM dependency on
customerio-gist-web
and replaces it with dynamic CDN loading - Adds a new build script for updating browser packages on CDP
- Downgrades Node.js version from 18.19.0 to 14.21.3
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
scripts/update_browser_packages_on_cdp.sh |
New build script for copying UMD assets to edge API |
packages/browser/src/plugins/in-app-plugin/index.ts |
Replaces static Gist import with dynamic CDN loading function |
packages/browser/src/plugins/in-app-plugin/__tests__/index.test.ts |
Updates tests to mock global Gist object and CDN loading behavior |
packages/browser/package.json |
Removes customerio-gist-web dependency |
.mise.toml |
Downgrades Node.js version to 14.21.3 |
|
||
// Create script tag to load from CDN | ||
const script = document.createElement('script') | ||
script.src = 'https://code.gist.build/web/latest/gist.min.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using 'latest' in the CDN URL can lead to unpredictable behavior and breaking changes. Consider pinning to a specific version for better stability and predictability.
script.src = 'https://code.gist.build/web/latest/gist.min.js' | |
// Pinned to a specific version for stability; update as needed | |
script.src = 'https://code.gist.build/web/1.0.0/gist.min.js' |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The src will change to a CIO DNS with a version dedicated to CDP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to CSP and our docs, we can't use the new CIO domains. The code.gist.build domain is already in our CSP docs so we should be fine.
Instead of relying on our NPM package, we’re now pulling the latest version of the in-app plugin directly from our CDN.
We cannot merge this PR until the Gist in-app SDK is served through CIO's assets domain.
Part of: INAPP-13803