feat: support configurable deployment basepath #158
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is a typosquat?Package name is similar to other popular packages and may not be the package you want. Use care when consuming similarly named packages and ensure that you did not intend to consume a different package. Malicious packages often publish using similar names as existing popular packages. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
feugy
force-pushed
the
damien/log-766-implement-web-analyticsspeed-insights-randomised-intake
branch
from
a809cb5
to
2f29274
Compare
feugy
force-pushed
the
damien/log-766-implement-web-analyticsspeed-insights-randomised-intake
branch
from
2f29274
to
4a65858
Compare
feugy
force-pushed
the
damien/log-766-implement-web-analyticsspeed-insights-randomised-intake
branch
from
4a65858
to
705a0f0
Compare
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment, @typescript-eslint/prefer-ts-expect-error -- only TS during build time will complain. | ||
| // @ts-ignore -- Rollup will fail to generate d.ts because it doesn't know import.meta.env. | ||
| const viteValue = import.meta.env | ||
| .VITE_VERCEL_OBSERVABILITY_BASEPATH as string; |
There was a problem hiding this comment.
should we also check for typeof import === 'undefined' like for projectId?
There was a problem hiding this comment.
No we don't have to, because import doesn't behave like process. process is a variable that belongs to the global namespace, while import is a keyword.
So it doesn't have a type, that's why I've wrapped it within a try-catch.
feugy
force-pushed
the
damien/log-766-implement-web-analyticsspeed-insights-randomised-intake
branch
from
a450c58
to
23388cb
Compare
🖖 What's in there?
Since ages, we're considering the possibility to configure, at build time, the basepath used for both downloading the injected script and reporting the collected metrics.
This PR brings such capability, and works with our supported frameworks: plain React (CRA), Next, Nuxt, Vue, Sveltkit, Astro, Remix (only when used with vite).
🤺 How to test?
Check the vercel deployments of each example app.
To reproduce locally, and set custom basepath:
pnpm i && pnpm -F @vercel/analytics buildThen:
For Nextjs app
NEXT_PUBLIC_VERCEL_OBSERVABILITY_BASEPATH="/abc" pnpm -F nextjs build pnpm -F nextjs startbrowse to http://localhost:3000 and check your console: it's trying (and failing) to download
/abc/insights/script.jsFor Nextjs 15 app
NEXT_PUBLIC_VERCEL_OBSERVABILITY_BASEPATH="/def" pnpm -F nextjs-15 build pnpm -F nextjs-15 startbrowse to http://localhost:3000 and check your console: it's trying (and failing) to download
/def/insights/script.jsFor Sveltekit app
VITE_VERCEL_OBSERVABILITY_BASEPATH="/hij" pnpm -F sveltekit build pnpm -F sveltekit previewbrowse to http://localhost:4173 and check your console: it's trying (and failing) to download
/hij/insights/script.jsFor vue app
VITE_VERCEL_OBSERVABILITY_BASEPATH="/klm" pnpm -F vue build pnpm -F vue previewbrowse to http://localhost:4173 and check your console: it's trying (and failing) to download
/klm/insights/script.jsFor nuxt app
VITE_VERCEL_OBSERVABILITY_BASEPATH="/nop" pnpm -F nuxt build pnpm -F nuxt previewbrowse to http://localhost:3000 and check your console: it's trying (and failing) to download
/nop/insights/script.jsFor remix app
VITE_VERCEL_OBSERVABILITY_BASEPATH="/qrs" pnpm -F remix build pnpm -F remix startbrowse to http://localhost:3000 and check your console: it's trying (and failing) to download
/qrs/insights/script.jsFor astro app
PUBLIC_VERCEL_OBSERVABILITY_BASEPATH="/tuv" pnpm -F astro build pnpm -F astro previewbrowse to http://localhost:4321 and check your console: it's trying (and failing) to download
/tuv/insights/script.jsThere is no example for CRA.
🔬Notes to reviewers
This is a generic solution, heavily inspired by @emspishak (thanks for the work!), and would replace #156.
I've changed the example app for Remix to used the vite, as per the latest official recommendation (remix 2.14 to date).
I've also replaced Jest with vitest, because the primer couldn't handle
import.metaproperly. And it's superior in my opinion (look at the reduced number of deps and shorter configuration).🔗 Related PRs