Skip to content

fix: use deep comparison for launch params to prevent spurious warning#142

Draft
behnam-oneschema wants to merge 1 commit intomainfrom
devin/1773954435-fix-spurious-launch-warning
Draft

fix: use deep comparison for launch params to prevent spurious warning#142
behnam-oneschema wants to merge 1 commit intomainfrom
devin/1773954435-fix-spurious-launch-warning

Conversation

@behnam-oneschema
Copy link
Copy Markdown
Member

Summary

Fixes #137. The OneSchemaImporter React component logs a spurious "already launched" warning on every re-render after the importer has launched, even when no launch params have actually changed.

Root cause: params is created via ...rest destructuring in the function signature, producing a new object reference on every render. The useEffect dependency array used this unstable reference, so the effect (and the warning) fired on every render cycle.

Fix: Replace referential equality with JSON.stringify-based deep comparison. A useRef tracks the previous serialized params, and the warning only fires when param values have genuinely changed. The dependency array uses JSON.stringify(params) so the effect itself is also skipped when values are stable.

Review & Testing Checklist for Human

  • Verify that JSON.stringify is adequate for all OneSchemaLaunchParamOptions fields — confirm none contain functions, undefined values that matter, or circular references that would break serialization
  • Confirm the first-render guard (prevParamsRef.current !== undefined) correctly suppresses the warning on initial mount (we don't want a warning before params have ever "changed")
  • Manual test: Render <OneSchemaImporter> with isOpen={true}, trigger a parent re-render (e.g. unrelated state change), and confirm the console warning no longer appears. Then actually change a launch param (e.g. userJwt) and confirm the warning does appear.

Notes

  • No new dependencies added; JSON.stringify is used instead of a third-party deep-compare library.
  • The eslint-disable-next-line react-hooks/exhaustive-deps is intentional: the dep array uses a computed JSON.stringify(params) expression rather than the raw params object, which the lint rule doesn't understand.

Link to Devin session: https://app.devin.ai/sessions/d9dbde3b76454bd6aa82a610c90d6776
Requested by: @behnam-oneschema

The `params` object created via rest destructuring (`...rest`) gets a new
reference on every render, causing the 'already launched' warning useEffect
to fire on every re-render — even when no param values actually changed.

Replace referential equality with JSON.stringify-based deep comparison using
a useRef to track the previous serialized params value. The warning now only
fires when param values have genuinely changed after the importer has already
launched.

Closes #137

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

Original prompt from Behnam 🧑‍💻

propose a fix for #137

@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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.

The "The OneSchema importer has already launched" warning doesn't work properly

1 participant