Conversation
There was a problem hiding this comment.
Code Review
This pull request updates documentation and rule files to include guidelines for environment variable validation and bumps the build-ts dependency along with several other packages. Feedback was provided to replace the recommendation of using the Node.js assert module with throwing a standard Error to ensure compatibility across different environments, including browsers.
| - Prefer `undefined` over `null` unless explicitly required by APIs or libraries. | ||
| - Prefer using a single template literal for prompts instead of `join()` with an array of strings. | ||
| - Assume that all environment variables are properly defined. | ||
| - If validation is required, use `assert` to fail fast (e.g., during startup). |
There was a problem hiding this comment.
Using the Node.js assert module for validation is problematic in a React library intended for the browser, as it will cause bundling issues or runtime errors. Consider recommending a more portable approach, such as throwing a standard Error to fail fast.
- If validation is required, throw an Error to fail fast (e.g., during startup).
| - Prefer `undefined` over `null` unless explicitly required by APIs or libraries. | ||
| - Prefer using a single template literal for prompts instead of `join()` with an array of strings. | ||
| - Assume that all environment variables are properly defined. | ||
| - If validation is required, use `assert` to fail fast (e.g., during startup). |
There was a problem hiding this comment.
Using the Node.js assert module for validation is problematic in a React library intended for the browser, as it will cause bundling issues or runtime errors. Consider recommending a more portable approach, such as throwing a standard Error to fail fast.
| - If validation is required, use `assert` to fail fast (e.g., during startup). | |
| - If validation is required, throw an Error to fail fast (e.g., during startup). |
| - Prefer `undefined` over `null` unless explicitly required by APIs or libraries. | ||
| - Prefer using a single template literal for prompts instead of `join()` with an array of strings. | ||
| - Assume that all environment variables are properly defined. | ||
| - If validation is required, use `assert` to fail fast (e.g., during startup). |
There was a problem hiding this comment.
Using the Node.js assert module for validation is problematic in a React library intended for the browser, as it will cause bundling issues or runtime errors. Consider recommending a more portable approach, such as throwing a standard Error to fail fast.
| - If validation is required, use `assert` to fail fast (e.g., during startup). | |
| - If validation is required, throw an Error to fail fast (e.g., during startup). |
| - Prefer `undefined` over `null` unless explicitly required by APIs or libraries. | ||
| - Prefer using a single template literal for prompts instead of `join()` with an array of strings. | ||
| - Assume that all environment variables are properly defined. | ||
| - If validation is required, use `assert` to fail fast (e.g., during startup). |
There was a problem hiding this comment.
Using the Node.js assert module for validation is problematic in a React library intended for the browser, as it will cause bundling issues or runtime errors. Consider recommending a more portable approach, such as throwing a standard Error to fail fast.
| - If validation is required, use `assert` to fail fast (e.g., during startup). | |
| - If validation is required, throw an Error to fail fast (e.g., during startup). |
No description provided.