-
Notifications
You must be signed in to change notification settings - Fork 548
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.env
variables inwebpack.config.cjs
usingDefinePlugin
- Adds
"node"
types intsconfig.json
and refinesclientProps.ts
to cast env vars - Updates README and
.env.template
to 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
@@ -10,7 +10,7 @@ All the code required to set up the Fluid Framework and SharedTree data structur | |||
|
|||
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 |
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 |
Copilot uses AI. Check for mistakes.
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.