Skip to content

Conversation

N2D4
Copy link
Contributor

@N2D4 N2D4 commented Apr 22, 2025


Important

Update ESLint config paths and enhance build process with tsup across the project.

  • ESLint Configuration:
    • Update ESLint config paths from ../../eslint-configs/ to ../../configs/eslint/ in various .eslintrc files across apps, examples, and packages.
    • Consolidate ESLint rules and ignore patterns in .eslintrc files.
  • Build Configuration:
    • Introduce configs/tsup/js-library.ts and configs/tsup/plugins.ts for shared tsup configuration.
    • Update tsup.config.ts in packages/stack-shared and packages/template to use createJsLibraryTsupConfig.
    • Add esbuild as a dev dependency in package.json.
  • Dockerfile:
    • Add COPY configs ./configs in docker/server/Dockerfile to include new config paths.
  • Miscellaneous:
    • Update package.json scripts and dependencies to reflect new build and linting configurations.
    • Modify import paths in api-keys.tsx and promises.tsx in packages/stack-shared to use relative paths.

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

@N2D4 N2D4 requested a review from bazumo April 22, 2025 23:07
Copy link

vercel bot commented Apr 22, 2025

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

Name Status Preview Comments Updated (UTC)
stack-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 29, 2025 4:31am
stack-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 29, 2025 4:31am
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 29, 2025 4:31am

Copy link

recurseml bot commented Apr 22, 2025

⚠️ Only 5 files will be analyzed due to processing limits.

Copy link

recurseml bot commented Apr 22, 2025

⚠️ Only 5 files will be analyzed due to processing limits.

Copy link

recurseml bot commented Apr 22, 2025

✨ No issues found! Your code is sparkling clean! ✨

🗒️ 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
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 reorganizes the build system for stack-shared and consolidates ESLint configurations across the repository.

  • Moves ESLint configs from /eslint-configs/ to /configs/eslint/ directory, updating paths across all packages
  • Introduces new configs/tsup/js-library.ts for shared build configuration with custom plugins for 'use client' directives
  • Switches stack-shared from tsc to tsup-node build system with both ESM/CJS output formats
  • Fixes import paths in stack-shared from absolute to relative imports for better compatibility
  • Missing tsup-node in devDependencies despite being used in build scripts

💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!

25 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

@N2D4 N2D4 merged commit c25abaa into dev Apr 29, 2025
8 of 15 checks passed
@N2D4 N2D4 deleted the stack-shared-tsup branch April 29, 2025 04:26

// make sure "use client" is at the top of the file
const matchUseClient = /[\s\n\r]*(^|\n|\r|;)\s*['"]use\s+client['"]\s*(\n|\r|;)/im;
if (matchUseClient.test(file.text)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Regex in onEnd callback prepends 'use client' even if already present at the top, potentially duplicating the directive.

Suggested change
if (matchUseClient.test(file.text)) {
if (!matchUseClient.test(file.text)) {

import path from "path";

export const createBasePlugin = (options: { customNoExternal: Set<string> }): Plugin => {
const packageJson = JSON.parse(fs.readFileSync("./package.json", "utf-8"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Reading package.json synchronously may be fragile if the working directory changes; consider ensuring the correct base path.

Suggested change
const packageJson = JSON.parse(fs.readFileSync("./package.json", "utf-8"));
const packageJson = JSON.parse(fs.readFileSync(path.resolve(__dirname, "package.json"), "utf-8"));

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.

2 participants