Skip to content

Update build error #1755

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

Merged
merged 1 commit into from
Apr 8, 2025
Merged

Update build error #1755

merged 1 commit into from
Apr 8, 2025

Conversation

Kitenite
Copy link
Contributor

@Kitenite Kitenite commented Apr 8, 2025

Description

Related Issues

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes


Important

Update next.config.ts to ignore TypeScript build errors and add custom error handling in _error.js.

  • Configuration:
    • In next.config.ts, set ignoreBuildErrors: true to allow production builds to complete despite TypeScript errors.
  • Error Handling:
    • Add _error.js to handle server and client errors with a custom message based on statusCode.

This description was created by Ellipsis for a6aeab0. It will automatically update as commits are pushed.

@Kitenite Kitenite merged commit ca76a66 into main Apr 8, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to a6aeab0 in 1 minute and 22 seconds

More details
  • Looked at 43 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. apps/web/client/next.config.ts:9
  • Draft comment:
    Be cautious with ignoring TS build errors in production.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50%
    This comment is cautionary and does not provide a specific suggestion or actionable feedback. It falls under the category of asking the author to ensure something, which is against the rules.
2. apps/web/client/src/pages/_error.js:1
  • Draft comment:
    Consider using a TS file (_error.tsx) for consistency if the rest of the project uses TypeScript.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    Without seeing other files in the project, we can't verify if TypeScript is actually used consistently. The comment is speculative with "if the rest of the project uses TypeScript". Making assumptions about project-wide conventions without evidence violates our review principles.
    The comment could be valuable if the project does use TypeScript consistently. Converting to TypeScript would add type safety.
    However, we must stick to our principle of only keeping comments when we have strong evidence. We don't have evidence about TypeScript usage in the project.
    Delete the comment since we don't have concrete evidence about TypeScript usage in the rest of the project.
3. apps/web/client/next.config.ts:10
  • Draft comment:
    Using ignoreBuildErrors: true can mask critical type errors in production. Consider alternatives such as stricter type checking or conditional application of this override.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The author has explicitly added warning comments showing they are aware of the risks. The config change appears intentional and documented. The comment doesn't provide new information beyond what's already in the code comments. Suggesting alternatives without specifics isn't very actionable.
    The comment does raise valid concerns about type safety in production. Maybe the author needs to be warned about the risks.
    The author has already acknowledged the risks with explicit warning comments in the code. If this was unintentional, they wouldn't have added those warnings.
    Delete the comment since it doesn't provide new information beyond the existing warning comments and the change appears to be intentional.
4. apps/web/client/src/pages/_error.js:1
  • Draft comment:
    Consider migrating _error.js to TypeScript (e.g., _error.tsx) for consistency, as the rest of the project uses TypeScript.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    While TypeScript could add type safety, we don't have clear evidence that the rest of the project uses TypeScript. The comment makes an assumption without proof. Even if true, this kind of consistency suggestion should probably be handled through project-wide standards rather than PR comments.
    The comment could be valid if there's strong evidence of TypeScript usage elsewhere. Converting to TypeScript could catch potential type errors.
    Without concrete evidence of TypeScript usage in the project, this comment is speculative. The file works as-is and the change doesn't introduce any type-related issues.
    Delete the comment as it makes assumptions about project standards without evidence and doesn't address any immediate issues with the code changes.
5. apps/web/client/src/pages/_error.js:5
  • Draft comment:
    The error messages are currently 'An error ${statusCode} occurred on server' and 'An error occurred on client'. Consider adding the definite article to improve the phrasing, e.g., 'An error ${statusCode} occurred on the server' and 'An error occurred on the client'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is a UI text change suggestion. Our rules explicitly state not to comment on pure frontend UI changes. While the suggestion might make the text slightly more grammatically correct, it's a minor stylistic choice that doesn't affect functionality. The current phrasing is perfectly understandable.
    The text change could improve user experience and readability, which might be considered more than just UI styling.
    Our rules are clear about not commenting on UI and styling changes. Error message phrasing falls under UI concerns, and we should trust the author's judgment on such matters.
    Delete this comment as it violates our rule about not commenting on UI/styling changes.

Workflow ID: wflow_Oux26l934XyLxLq8


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@Kitenite Kitenite deleted the refactor/web-build-1 branch April 9, 2025 01:02
ml-delaurier pushed a commit to ml-delaurier/nolook that referenced this pull request Apr 23, 2025
t1c1 pushed a commit to t1c1/onlookbotcodes that referenced this pull request Jun 5, 2025
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