-
Notifications
You must be signed in to change notification settings - Fork 403
fix: update example to work with CodeSandbox and other cloud sandboxes #730
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?
Changes from all commits
b454064
a64863c
5ab4471
2f51b44
d8dbd04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| { | ||
| "setupTasks": [ | ||
| { | ||
| "name": "Install dependencies", | ||
| "command": "npm install" | ||
| } | ||
| ], | ||
| "tasks": { | ||
| "dev": { | ||
| "name": "Start dev server", | ||
| "command": "npm start", | ||
| "runAtStart": true, | ||
| "preview": { | ||
| "port": 5173 | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,23 +9,23 @@ | |||||
| }, | ||||||
| "dependencies": { | ||||||
| "firebase": "^9.0.0", | ||||||
| "react": "^17.0.0", | ||||||
| "react-dom": "^17.0.0", | ||||||
| "reactfire": "file:../reactfire-4.0.1.tgz", | ||||||
| "react": "^18.0.0", | ||||||
| "react-dom": "^18.0.0", | ||||||
| "reactfire": "latest", | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using "latest" for a dependency is discouraged because it makes builds non-deterministic and can break the example in the future if a new major version of reactfire is released with breaking changes. It is highly recommended to pin this to a specific version or a safe semver range (e.g., "^4.2.3" which is the current stable version).
Suggested change
|
||||||
| "rollup-plugin-visualizer": "^5.7.1" | ||||||
| }, | ||||||
| "browserslist": [ | ||||||
| "last 1 Chrome version" | ||||||
| ], | ||||||
| "devDependencies": { | ||||||
| "@types/react": "^18.0.15", | ||||||
| "@types/react-dom": "^18.0.6", | ||||||
| "@vitejs/plugin-react": "^2.0.0", | ||||||
| "@types/react": "^18.0.0", | ||||||
| "@types/react-dom": "^18.0.0", | ||||||
| "@vitejs/plugin-react": "^4.0.0", | ||||||
| "autoprefixer": "^10.4.8", | ||||||
| "babel-preset-env": "^1.7.0", | ||||||
| "postcss": "^8.4.14", | ||||||
| "tailwindcss": "^3.1.7", | ||||||
| "typescript": "^4.7.4", | ||||||
| "vite": "^3.2.7" | ||||||
| "typescript": "^5.0.0", | ||||||
| "vite": "^5.0.0" | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server.allowedHosts configuration option was introduced in Vite 5.4.12 (as a backport for the security fix) and Vite 6.0.9. Since you are specifying "vite": "^5.0.0", npm could resolve this to an older version of Vite 5 (e.g., 5.0.0 to 5.4.11) which does not support this option or include the security fix. To ensure the option is recognized and the security fix is active, please bump the minimum Vite version to "^5.4.12".
Suggested change
|
||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,4 +10,7 @@ export default defineConfig({ | |||||
| // Helps make sure we aren't pulling in extra deps | ||||||
| visualizer({ template: 'treemap' }), | ||||||
| ], | ||||||
| server: { | ||||||
| allowedHosts: true, | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting allowedHosts: true disables host validation entirely, which exposes the local development server to DNS rebinding attacks (as warned in Vite's documentation). Since this example is intended for cloud sandboxes like CodeSandbox and StackBlitz, you can restrict the allowed hosts to only these platforms using subdomain wildcards (by prefixing with a dot "."). This maintains compatibility while keeping the dev server secure.
Suggested change
|
||||||
| }, | ||||||
| }); | ||||||
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.
Upgrading react and react-dom to "^18.0.0" is great, but the entry point (example/index.tsx) still uses the deprecated ReactDOM.render API. This will cause React 18 to run in React 17 backward-compatibility mode and log a deprecation warning in the console. To fully leverage React 18, the entry point should be updated to use createRoot from react-dom/client.