Skip to content

Revert "Demo floating window"#692

Merged
N2D4 merged 1 commit into
devfrom
revert-678-demo-floating-window
May 20, 2025
Merged

Revert "Demo floating window"#692
N2D4 merged 1 commit into
devfrom
revert-678-demo-floating-window

Conversation

@N2D4
Copy link
Copy Markdown
Contributor

@N2D4 N2D4 commented May 20, 2025

Reverts #678


Important

Reverts the addition of the DemoFloatingWindow component by removing its references and deleting its file.

  • Revert Demo Floating Window:
    • Remove DemoFloatingWindow component from layout.tsx.
    • Delete demo-floating-window.tsx component file.
    • Remove DemoFloatingWindow import and usage in init-stack/src/index.ts.
    • Remove DemoFloatingWindow export from template/src/index.ts.
  • Misc:
    • Adjust import statements in layout.tsx and init-stack/src/index.ts to exclude DemoFloatingWindow.

This description was created by Ellipsis for 4c1f6d0. You can customize this summary. It will automatically update as commits are pushed.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-backend 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 20, 2025 0:56am
stack-dashboard 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 20, 2025 0:56am
stack-demo 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 20, 2025 0:56am

@N2D4 N2D4 merged commit c19bb3a into dev May 20, 2025
13 of 17 checks passed
@N2D4 N2D4 deleted the revert-678-demo-floating-window branch May 20, 2025 00:56

const insertOpen = "<StackProvider app={stackServerApp}><StackTheme><DemoFloatingWindow />";
const lines = layout.split("\n");
const [bodyOpenEndLine, bodyOpenEndIndexInLine] = getLineIndex(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code could crash when calculating line indices if the body tag's position (bodyOpenEndIndex) extends beyond the total length of concatenated lines. The getLineIndex function throws an error in this case, which isn't caught by the caller. This can happen with malformed HTML or when body tags span multiple lines. The function should handle this edge case gracefully instead of throwing an error.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

export { UserButton } from "./components/user-button";

export { DemoFloatingWindow } from "./components/demo-floating-window";
export { CliAuthConfirmation } from "./components-page/cli-auth-confirm";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This export statement references a non-existent module. The component 'CliAuthConfirmation' and its containing module './components-page/cli-auth-confirm' appear to be missing from the codebase. This will cause a module resolution error when any consumer tries to import this component. The bug was introduced by keeping an export for a module that doesn't exist in the project structure.


React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)

@recurseml
Copy link
Copy Markdown

recurseml Bot commented May 20, 2025

😱 Found 2 issues. Time to roll up your sleeves! 😱

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
  • Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

  • Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
  • The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

  • Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
  • The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

  • 'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

  • When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR reverts the addition of the DemoFloatingWindow component, a development-only feature that provided quick access to authentication actions.

  • Removes packages/template/src/components/demo-floating-window.tsx component and its export from packages/template/src/index.ts
  • Removes DemoFloatingWindow integration from examples/demo/src/app/layout.tsx
  • Updates packages/init-stack/src/index.ts to remove DemoFloatingWindow from generated layout templates
  • Relocates CliAuthConfirmation export in packages/template/src/index.ts to maintain organization

4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant