Skip to content

landing 1 base proto review need ,cleanup need#2896

Closed
ArnavK-09 wants to merge 10 commits intomainfrom
ddd1
Closed

landing 1 base proto review need ,cleanup need#2896
ArnavK-09 wants to merge 10 commits intomainfrom
ddd1

Conversation

@ArnavK-09
Copy link
Copy Markdown
Collaborator

No description provided.

@ArnavK-09 ArnavK-09 changed the title landing 1 base proto review need landing 1 base proto review need ,cleanup need Mar 2, 2026
@seveibar
Copy link
Copy Markdown
Contributor

seveibar commented Mar 2, 2026

Avoid changing copy, the title styling change is nice but i really need to see placeholders filled

@seveibar
Copy link
Copy Markdown
Contributor

seveibar commented Mar 2, 2026

The new landing title for example is really bad, i dont want to argue copy at the same time as design

@seveibar
Copy link
Copy Markdown
Contributor

seveibar commented Mar 2, 2026

(noticing improvements, a bit happier ;)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2026

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Mar 6, 2026
@ArnavK-09 ArnavK-09 removed the stale label Mar 7, 2026
@ArnavK-09
Copy link
Copy Markdown
Collaborator Author

unstale

Comment on lines +75 to +103
useEffect(() => {
if (!containerRef.current) return

const startState = EditorState.create({
doc: code,
extensions: [
basicSetup,
javascript({ typescript: true, jsx: true }),
darkTheme,
syntaxHighlighting(githubDarkHighlightStyle),
EditorView.updateListener.of((update) => {
if (update.docChanged) {
onChange(update.state.doc.toString())
}
}),
],
})

const view = new EditorView({
state: startState,
parent: containerRef.current,
})

viewRef.current = view

return () => {
view.destroy()
}
}, [])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Critical stale closure bug: The onChange callback is captured in the closure when the editor is initialized (line 87), but the effect has an empty dependency array (line 103). If the parent component re-renders and passes a new onChange function, the editor will continue using the old captured onChange, causing updates to be sent to stale/incorrect handlers.

This will break when:

  • State updates don't reflect in the parent
  • The parent component re-renders with new state
  • Multiple instances share stale callbacks

Fix: Use a ref to always access the latest onChange:

const onChangeRef = useRef(onChange)
const viewRef = useRef<EditorView | null>(null)

useEffect(() => {
  onChangeRef.current = onChange
}, [onChange])

useEffect(() => {
  if (!containerRef.current) return

  const startState = EditorState.create({
    doc: code,
    extensions: [
      basicSetup,
      javascript({ typescript: true, jsx: true }),
      darkTheme,
      syntaxHighlighting(githubDarkHighlightStyle),
      EditorView.updateListener.of((update) => {
        if (update.docChanged) {
          onChangeRef.current(update.state.doc.toString())
        }
      }),
    ],
  })
  // ...
}, [])

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

@github-actions
Copy link
Copy Markdown

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Mar 13, 2026
@github-actions
Copy link
Copy Markdown

This PR was closed because it has been inactive for 1 day since being marked as stale.

@github-actions github-actions bot closed this Mar 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants