Closed jeremybanka closed 2 months ago
vercel[bot]
commented
2 months ago The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| atom-io-fyi | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Aug 28, 2024 2:54am |
| wayfarer-quest | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Aug 28, 2024 2:54am |
changeset-bot[bot]
commented
2 months ago Latest commit: adb8cca8efc515226c8edc0340160e507d6284f8
The changes in this PR will be included in the next version bump.
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
github-actions[bot]
commented
2 months ago | ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪ |
| 🧪 PR contains tests |
| 🔒 No security concerns identified |
| ⚡ Key issues to review Refactoring Issue The refactoring of `isDone` and `markDone` functions changed the logging level from `warn` to `error`. This change could potentially lead to a different handling of operational logs which might not be intended. Ensure that this change aligns with the intended error handling and logging strategy. Schema Removal The removal of the `schema` property from `JsonEditor` might affect the validation or structure checks that were previously in place. Verify that removing schema handling does not affect the functionality where JSON data structure validation is critical. Test Modification The test for `ReactErrorBoundary` has been simplified by removing error count checks. This change might reduce the thoroughness of the test. Consider the implications of not checking the exact number of errors expected during the component's operation. |
github-actions[bot]
commented
2 months ago | Category | Suggestion | Score |
| Possible bug |
Add error handling for number conversion to prevent runtime errors___ **Consider adding error handling for theNumber constructor in the stringToNumber function to handle cases where the string is not a valid number. This can prevent potential runtime errors due to NaN values.**
[packages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/cast-json.ts [4]](https://github.com/jeremybanka/wayforge/pull/2460/files#diff-254c074ac26a3bb6ca05f497eee8a9a9d8f3f34ba74d626280bbe0dee39b4ef6R4-R4)
```diff
-export const stringToNumber = (str: string): number => Number(str)
+export const stringToNumber = (str: string): number => {
+ const num = Number(str);
+ if (isNaN(num)) {
+ throw new Error(`Invalid number: ${str}`);
+ }
+ return num;
+}
```
Suggestion importance[1-10]: 9Why: The suggestion correctly identifies a potential runtime error when converting strings to numbers. Adding error handling for invalid number conversions is crucial to prevent unexpected behavior and improve robustness. | 9 |
| Possible issue |
Handle cases where number conversion might result in NaN___ **Refactor theobjectToNumber function to handle cases where none of the properties ( number, size, count) are present, to avoid returning NaN.**
[packages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/cast-json.ts [18-19]](https://github.com/jeremybanka/wayforge/pull/2460/files#diff-254c074ac26a3bb6ca05f497eee8a9a9d8f3f34ba74d626280bbe0dee39b4ef6R18-R19)
```diff
-export const objectToNumber = (obj: Json.Tree.Object): number =>
- Number(obj.number ?? obj.size ?? obj.count ?? 0)
+export const objectToNumber = (obj: Json.Tree.Object): number => {
+ const result = Number(obj.number ?? obj.size ?? obj.count);
+ return isNaN(result) ? 0 : result;
+}
```
Suggestion importance[1-10]: 9Why: The suggestion addresses a potential issue where the function could return `NaN`, which is undesirable. Handling this case improves the function's reliability and correctness. | 9 |
| Enhancement |
Improve error logging for JSON parsing failures___ **In thestringToObject function, consider providing a more informative error message in the catch block to aid debugging when JSON parsing fails.** [packages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/cast-json.ts [6-11]](https://github.com/jeremybanka/wayforge/pull/2460/files#diff-254c074ac26a3bb6ca05f497eee8a9a9d8f3f34ba74d626280bbe0dee39b4ef6R6-R11) ```diff try { return JSON.parse(str) } catch (e) { + console.error(`Failed to parse string as JSON: ${str}`, e); return { [str]: str } } ``` Suggestion importance[1-10]: 8Why: Improving error logging for JSON parsing failures is beneficial for debugging and maintenance, making it easier to identify and resolve issues. | 8 |
| Performance |
Avoid unnecessary property renaming operations___ **In themakePropertyRenamers function, consider checking if newKey is different from key before proceeding with renaming to avoid unnecessary operations.**
[packages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/object-properties.ts [30-41]](https://github.com/jeremybanka/wayforge/pull/2460/files#diff-df0adef5b5334e5f5f7a1cf1e68bd3a2eb327c1954f454f7d59e342882fa24e8R30-R41)
```diff
-if (!Object.hasOwn(data, newKey)) {
+if (key !== newKey && !Object.hasOwn(data, newKey)) {
set(() => {
const entries = Object.entries(data)
const index = entries.findIndex(([k]) => k === key)
entries[index] = [newKey, value]
const stableKeyMap = stableKeyMapRef.current
stableKeyMapRef.current = {
...stableKeyMap,
[newKey]: stableKeyMap[key],
}
return Object.fromEntries(entries) as T
})
}
```
Suggestion importance[1-10]: 7Why: The suggestion improves performance by avoiding unnecessary operations, which is a good practice for optimizing code efficiency, although it is not critical. | 7 |
coveralls
commented
2 months ago coverage: 92.419% (+0.4%) from 92.05% when pulling adb8cca8efc515226c8edc0340160e507d6284f8 on atom.io-consolidate-devtools into 1e33475a6ce10be4712cd97f035b4b1fce4932fb on main.
User description
PR Type
Enhancement, Tests
Description
ElasticInput,JsonEditor, and error boundaries.Changes walkthrough 📝
5 files
realtime-continuity.test.tsx
Increase test timeout for better reliabilitypackages/atom.io/__tests__/experimental/realtime/realtime-continuity.test.tsx - Increased test timeout to 10000 milliseconds.
grace.test.ts
Add tests for internal debugging warningspackages/atom.io/__tests__/private/grace.test.ts - Added tests for internal debugging warnings.
AtomIODevtools.test.tsx
Add tests for AtomIODevtools state and transaction viewspackages/atom.io/__tests__/private/introspection/AtomIODevtools.test.tsx
ReactErrorBoundary.test.tsx
Simplify ReactErrorBoundary test setuppackages/atom.io/__tests__/private/introspection/ReactErrorBoundary.test.tsx - Simplified error boundary test setup.
silo.test.ts
Add subscription tests for state changespackages/atom.io/__tests__/public/silo.test.ts - Added subscription tests for state changes.
5 files
sprawl.test.ts
Update import path for sprawlpackages/atom.io/__tests__/private/introspection/sprawl.test.ts - Updated import path for sprawl.
walk.ts
Remove unneeded case in walk functionpackages/atom.io/eslint-plugin/src/walk.ts - Removed unneeded case for FunctionDeclaration.
differ.ts
Update import path for sprawl in differpackages/atom.io/introspection/src/differ.ts - Changed import path for `sprawl`.
sprawl.ts
Update import path for isPlainObject in sprawlpackages/atom.io/introspection/src/sprawl.ts - Updated import path for `isPlainObject`.
test-utils.ts
Remove unused test utilitiespackages/hamr/react-error-boundary/src/test-utils.ts - Removed unused test utilities.
29 files
operation.ts
Refactor operation functions and improve loggingpackages/atom.io/internal/src/operation.ts
openOperation,isDone, andmarkDone.errorinstead ofwarn.create-writable-selector.ts
Update markDone function call in selectorpackages/atom.io/internal/src/selector/create-writable-selector.ts - Updated `markDone` function call with new parameter order.
become.ts
Simplify become function implementationpackages/atom.io/internal/src/set-state/become.ts - Simplified `become` function implementation.
evict-downstream.ts
Update function calls in evictDownStreampackages/atom.io/internal/src/set-state/evict-downstream.ts
isDoneandmarkDonefunction calls with new parameter order.set-atom.ts
Update markDone function call in setAtompackages/atom.io/internal/src/set-state/set-atom.ts - Updated `markDone` function call with new parameter order.
set-into-store.ts
Update openOperation function call in setIntoStorepackages/atom.io/internal/src/set-state/set-into-store.ts - Updated `openOperation` function call with new parameter order.
refinery.ts
Extract isPlainObject function for reusepackages/atom.io/introspection/src/refinery.ts - Extracted `isPlainObject` function for reuse.
index.ts
Add JSON type names and defaultspackages/atom.io/json/src/index.ts - Added JSON type names and defaults.
StateEditor.tsx
Update imports and remove schema prop in StateEditorpackages/atom.io/react-devtools/src/StateEditor.tsx
ElasticInputandJsonEditor.JsonEditor.ElasticInput.tsx
Add ElasticInput componentpackages/atom.io/react-devtools/src/elastic-input/ElasticInput.tsx - Added new `ElasticInput` component.
NumberInput.tsx
Refactor number input constraints and clampingpackages/atom.io/react-devtools/src/elastic-input/NumberInput.tsx - Refactored number input constraints and clamping logic.
TextInput.tsx
Add TextInput componentpackages/atom.io/react-devtools/src/elastic-input/TextInput.tsx - Added new `TextInput` component.
index.ts
Export components from elastic-inputpackages/atom.io/react-devtools/src/elastic-input/index.ts - Exported components from `elastic-input` directory.
DefaultFallback.tsx
Add DefaultFallback component for error boundariespackages/atom.io/react-devtools/src/error-boundary/DefaultFallback.tsx - Added `DefaultFallback` component for error boundaries.
ReactErrorBoundary.tsx
Add ErrorBoundary and RecoverableErrorBoundary componentspackages/atom.io/react-devtools/src/error-boundary/ReactErrorBoundary.tsx - Added `ErrorBoundary` and `RecoverableErrorBoundary` components.
index.ts
Export error boundary componentspackages/atom.io/react-devtools/src/error-boundary/index.ts - Exported error boundary components.
index.ts
Export error-boundary from indexpackages/atom.io/react-devtools/src/index.ts - Exported `error-boundary` from index.
default-components.tsx
Update ErrorBoundary import and Button propspackages/atom.io/react-devtools/src/json-editor/default-components.tsx
ErrorBoundaryimport path.Buttonprops optional.developer-interface.tsx
Remove schema-related code from developer interfacepackages/atom.io/react-devtools/src/json-editor/developer-interface.tsx - Removed schema-related code.
array-editor.tsx
Update ArrayEditor props typepackages/atom.io/react-devtools/src/json-editor/editors-by-type/array-editor.tsx - Updated type for `ArrayEditor` props.
non-json.tsx
Update ElasticInput import path in non-json editorpackages/atom.io/react-devtools/src/json-editor/editors-by-type/non-json.tsx - Updated import path for `ElasticInput`.
object-editor.tsx
Simplify object editor and remove schema logicpackages/atom.io/react-devtools/src/json-editor/editors-by-type/object-editor.tsx - Removed schema-related logic. - Simplified property handling.
primitive-editors.tsx
Update ElasticInput import path in primitive editorspackages/atom.io/react-devtools/src/json-editor/editors-by-type/primitive-editors.tsx - Updated import path for `ElasticInput`.
array-elements.ts
Update type for makeElementSetterspackages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/array-elements.ts - Updated type for `makeElementSetters`.
cast-json.ts
Add utility functions for casting JSONpackages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/cast-json.ts - Added utility functions for casting JSON.
cast-to-json.ts
Update castToJson function for unknown inputpackages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/cast-to-json.ts - Updated `castToJson` function to handle unknown input.
object-properties.ts
Refactor property utilities with fromEntriespackages/atom.io/react-devtools/src/json-editor/editors-by-type/utilities/object-properties.ts - Refactored property utility functions to use `fromEntries`.
index.ts
Add SetterOrUpdater typepackages/atom.io/react-devtools/src/json-editor/index.ts - Added `SetterOrUpdater` type.
json-editor-internal.tsx
Simplify JsonEditor_INTERNAL and remove schema logicpackages/atom.io/react-devtools/src/json-editor/json-editor-internal.tsx - Removed schema-related code. - Simplified component logic.
1 files
todo.md
Add TODO list for JSON editor improvementspackages/atom.io/react-devtools/src/json-editor/todo.md - Added TODO list for JSON editor improvements.
1 files
tsconfig.json
Update include paths in tsconfigpackages/anvl/tsconfig.json - Updated include paths in tsconfig.
1 files
pnpm-lock.yaml
Update pnpm lockfile with new dependenciespnpm-lock.yaml - Updated lockfile with new dependencies.