Add react example

[yoDon]

This PR adds a small React NextJS TypeScript example. The JSONEditorReact component in the examples/react/components folder could easily be extracted and turned into an npm package if desired.

[josdejong]

Thanks Don. I prefer to not add a React example project to the repository itself but keep it separate. I had a two React examples added to the predecessor of this library, jsoneditor, which is quite a pain: I still regularly get security warnings about outdated dependencies that I have to keep up to date.

I would love to link to a Nextjs example from the "Use", "Examples" section though. Would it be possible to put the example in a separate repository, or create a CodeSandbox instead?

[yoDon]

Are you ok adding the React example to the README-VANILLA.md in text form, similar to the way you have the Browser ES modules example)?

The revised commit has TypeScript and JavaScript examples of how to wrap the editor in a React component and shows how to use a dynamic import to disable server side rendering in NextJS 13.

[josdejong]

That's a good idea 👍

[kloon15]

I would like to chime in on this one, as i dont see the proposed example cleaning up the ref after the component unmounts. Not that im an expert i just adapted someone elses code but i assume it should be necessary (i also made a form input for Ant Design):

import { Form } from 'antd'
import { useEffect, useRef, useState } from 'react'
import {
    Content,
    JSONEditor,
    JSONEditorPropsOptional,
    OnChangeStatus,
    TextContent,
} from 'vanilla-jsoneditor'

import '@/style/jsoneditor.css'

interface JSONEditorMyProps extends JSONEditorPropsOptional {
    className?: string
}

export const VanillaJSONEditor: React.FC<JSONEditorMyProps> = (props) => {
    const refContainer = useRef<HTMLDivElement>(null)
    const refEditor = useRef<JSONEditor | null>(null)

    const { className: _, ...editorProps } = props

    useEffect(() => {
        // create editor
        // console.log('create editor', editorProps)
        refEditor.current = new JSONEditor({
            target: refContainer.current!,
            props: {}
        })

        return () => {
            // destroy editor
            if (refEditor.current) {
                // console.log('destroy editor')
                refEditor.current.destroy()
                refEditor.current = null
            }
        }
    }, [])

    useEffect(() => {
        // update props
        if (refEditor.current) {
            // console.log('update props', editorProps)
            refEditor.current.updateProps(editorProps)
        }
    }, [props])

    return (<div
        className={`vanilla-jsoneditor ${props.className}`}
        ref={refContainer}></div>)
}

interface JSONEditorInputProps {
    value?: Content
    onChange?: (newContent: Content,
        status: OnChangeStatus) => void
}

export const JSONEditorInput: React.FC<JSONEditorInputProps> = ({ value, onChange }) => {
    const defaultContent = { text: '' } as TextContent
    const [content, setContent] = useState<Content>(value ?? defaultContent)

    const { status } = Form.Item.useStatus()

    const handleEditorChange = (content: Content,
        previousContent: Content, status: OnChangeStatus) => {
        // console.log('Content was updated:', content)
        setContent(content)
        onChange?.(content, status)
    }

    return (<VanillaJSONEditor
        content={content}
        onChange={handleEditorChange}
        className={status && `vanilla-jsoneditor-status-${status}`}
    />)
}

CSS file:

.vanilla-jsoneditor {
    display: flex;
    flex: auto;
    border-width: 1px;
    border-style: solid;
    border-color: #d9d9d9;
    transition: all 0.2s;
    border: 1px solid #d9d9d9;
}

.vanilla-jsoneditor-status-error {
    border-color: #ff4d4f;
}

Thoughts?

[josdejong]

Good point. Neatly destroying the editor is essential, and handling property updates is a must too. I would like to keep the example a minimal React example and not include things like antd forms.

[kloon15]

Sure thing, i just added it as an example.

[yoDon]

@kloon15 I just added you to the repo in case you wanted to add your changes to the PR

[yoDon]

I do think we should keep a NextJS variant in the example, because the use of a dynamic import is likely to not be obvious to a lot of people, but having a vanilla react example is of course good too.

[josdejong]

Yes agree, the NextJS explanation will be helpful. Can you update the PR with neatly destroying the editor and updating props? It also looks like you added JSONEditorReact.jsx twice.

[yoDon]

If @kloon15 would like to add the destructor code, I think that's best as it records who spotted the need and made it happen in the commit (I also like the improvements made to the ref variable names). If not, I'm happy to jump in and do the work to fix the examples.

I did try to add a bit more clarity to the readme to explain that the first example is TypeScript and the second is JavaScript (both examples would need the destructor code, if we intend to keep the two variants in the readme).

[kloon15]

Question: theres already a react example with codesandbox in the main README. Shouldnt we just expand on it by adding the TS and NextJS examples?

[kloon15]

Anyway i fixed up both codes with the destructor and props update side effects.

[yoDon]

EDITED: clarified that importing types this way is ok, the issue is just with importing functions like toTextContent.

Including import { Content, OnChangeStatus, toTextContent } from 'vanilla-jsoneditor' in the NextJS Demo.tsx file unfortunately breaks the build with:

ReferenceError: document is not defined
    at file:///.../node_modules/vanilla-jsoneditor/index.js:26:14207
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:527:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:438:15)

> Build error occurred

The problem is NextJS does server-side rendering and the vanilla-jsoneditor code references browser-only js features that are not available during server-side rendering.

We can either change the NextJS Demo.tsx example to not use toTextContext() or we can intentionally keep it and add a NextJS dynamic import to show how to do that (which would complicate the example, but possibly in a useful way).

[kloon15]

Sorry i didnt know that, I personally think showing that in the example would be beneficial, since most people would probably run into that error.

Edit: we should clarify then that those are type only imports: import type { Content, OnChangeStatus } from 'vanilla-jsoneditor'

[josdejong]

Thanks for updating the examples @yoDon , they look perfect like this 👌 . I think it is indeed important to keep the dynamic import in the example, that is a problem something you will easily run into when using Next.js

Where to put the examples (https://github.com/josdejong/svelte-jsoneditor/pull/232#issuecomment-1455082180) is a good point @kloon15 . Both a sandbox and an example inside a readme has pros/cons. I suggest the following approach:

1. In README-VANILLA.md we can put basic examples of all major platforms (vanilla, react, next.js, vue, angular, ...)
2. These examples can optionally include a link to a CodeSandbox where you can try out the example
3. We change the Examples section in the README.md to have only two bullets: one for the Svelte examples, and one for "Vanilla, React, Next.js, Vue, Angular examples", README-VANILLA.md (aligned with the two npm packages).
4. It may be a good idea to add a section to README.md or README-VANILLA.md explaining in general that if you use SSR, that you most likely have to use a dynamic import. And the other way around: it would be nice if we could improve svelte-jsoneditor such that it does support server side rendering instead of throwing errors 😁 .

If you guys agree, I think we can merge this PR as-is and address the plan above separately.

[kloon15]

Waiting on @yoDon to fix the NextJS example on how to import toTextContent, or we can just revert it to JSON.stringify for the time being.

[josdejong]

@yoDon what do you think about the last issue with toTextContent pointed out by @kloon15 ?

[yoDon]

I've been swamped the past couple days but definitely will be back on this early next week. My read is we do want an example of how to use dynamic to import toTextContent. I'm thinking that should be straight forward to do, but I want to make sure whatever we put in the docs actually works.

[josdejong]

Thanks for the update Don, take it easy!

[yoDon]

Haven't forgotten about this - I'm still aiming to get time to wrap this up

[yoDon]

I've updated the readme to demonstrate how to wrap helper functions (like toTextContent) in a dynamically imported component to prevent server-side rendering from failing when it encounters the vanilla-jsoneditor files.

If @kloon15 happens to have a moment to look over the updated code, that would be fantastic (the previous code review notes have been very good), but if that's too much to ask I can confirm the example runs and functions as expected under recent versions of NextJS.

[josdejong]

Thanks for the update Don, it looks very clear and the explanation and comments helps understanding the context 👌. Can you please run npm run format to resolve the linting issue?

@kloon15 any more feedbacks?

[yoDon]

@josdejong npm run format pushed

[yoDon]

I think this should be good to merge

[josdejong]

👍 thanks again Don!