-
Notifications
You must be signed in to change notification settings - Fork 561
Fix the ODSP shared tree demo issue #24627
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
Fix the ODSP shared tree demo issue #24627
Conversation
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 ensures the shared-tree demo can read environment variables locally by injecting them via Webpack and updating related configs and docs.
- Injects
process.envvariables inwebpack.config.cjsusingDefinePlugin - Adds
"node"types intsconfig.jsonand refinesclientProps.tsto cast env vars - Updates README and
.env.templateto guide users on environment variable setup
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/service-clients/odsp-client/shared-tree-demo/webpack.config.cjs | Added DefinePlugin entries for SITE_URL, SPE_DRIVE_ID, SPE_CLIENT_ID, SPE_ENTRA_TENANT_ID |
| examples/service-clients/odsp-client/shared-tree-demo/tsconfig.json | Included "node" in types to support process.env |
| examples/service-clients/odsp-client/shared-tree-demo/src/clientProps.ts | Removed custom process declaration and added as string casts for env vars |
| examples/service-clients/odsp-client/shared-tree-demo/README.md | Revised instructions to use environment variables from .env.template |
| examples/service-clients/odsp-client/shared-tree-demo/.env.template | Renamed DRIVE_ID to SPE_DRIVE_ID for consistency |
examples/service-clients/odsp-client/shared-tree-demo/README.md
Outdated
Show resolved
Hide resolved
| You can run this example using the following steps: | ||
|
|
||
| 1. To kick off the example, update the credentials in the `clientProps.ts` file by replacing siteUrl, driveId, and clientId with your own. | ||
| 1. To kick off the example, update the environment variable by replacing siteUrl, driveId, clientId, entra id with your own. Please check detailed environment variable names in .env.template |
Copilot
AI
May 14, 2025
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.
[nitpick] Capitalize and clarify 'entra id' to 'Entra Tenant ID' to match the variable SPE_ENTRA_TENANT_ID and maintain consistency.
| 1. To kick off the example, update the environment variable by replacing siteUrl, driveId, clientId, entra id with your own. Please check detailed environment variable names in .env.template | |
| 1. To kick off the example, update the environment variable by replacing siteUrl, driveId, clientId, Entra Tenant ID with your own. Please check detailed environment variable names in .env.template |
examples/service-clients/odsp-client/shared-tree-demo/webpack.config.cjs
Outdated
Show resolved
Hide resolved
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
Description
The example cannot get correct environment example for local run. Fix it by adding plugin in webpack config. The reason is webpack doesn't load environment variables by default. It only works in Node environment.