Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(examples): update react app code #5545

Merged
merged 2 commits into from Aug 3, 2023
Merged

fix(examples): update react app code #5545

merged 2 commits into from Aug 3, 2023

Conversation

epreston
Copy link
Contributor

@epreston epreston commented Aug 3, 2023

Update react code as outlined here to remove compatibility mode warnings.
https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-client-rendering-apis

Update react types for stricter checking and fixes.

Fixes react running in compatibility mode.

I confirm I have read the contributing guidelines and signed the Contributor License Agreement.

react dom is running in 17 compatibility mode with warnings.  This completes the required changes in the upgrade guide.
these definitions contain some corrections
@epreston
Copy link
Contributor Author

epreston commented Aug 3, 2023

This is said to have a perf benefit as well but I can't tell. Its always been very performant.

@epreston
Copy link
Contributor Author

epreston commented Aug 3, 2023

Completed a code scan with the react 18 and react 19 presets using the official upgrade tool.
See: https://react.dev/blog/2022/03/08/react-18-upgrade-guide#updates-to-typescript-definitions

Tool: https://github.com/eps1lon/types-react-codemod

It suggested no modifications.

@epreston epreston marked this pull request as ready for review August 3, 2023 18:30
Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

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

Nice, thanks!!

@willeastcott willeastcott merged commit 27fcf0c into playcanvas:main Aug 3, 2023
7 checks passed
@epreston epreston deleted the examples-react-dom branch August 3, 2023 19:19
@kungfooman
Copy link
Collaborator

Did you test the examples?

I changed this myself too (with many other changes) and I observed some examples don't work properly afterwards:

function main() {
    const oldWay = !true;
    console.log("oldWay", oldWay);
    if (oldWay) {    
        // render out the app
        ReactDOM.render(
            jsx(MainLayout, null),
            document.getElementById('app')
        );
    } else {
        // render out the app
        const container = document.getElementById('app');
        const root = createRoot(container);
        root.render(jsx(MainLayout, null));
    }
}
window.onload = () => {
    // Just a little timeout to give browser some time to "breathe"
    setTimeout(main, 50);
};

For example a slider doesn't fire any longer or this canvas is just half-visible:

image

Based on this minified source, it doesn't seem to be updated yet: https://playcanvas.github.io/index.js (I only find createRoot once)

@epreston
Copy link
Contributor Author

epreston commented Aug 11, 2023

Yes. All directional controls work.

I would not rely on what is published to github pages in the link provided. That is not the branch these commits have been made to. I do not know which version is being pushed to the "github pages".

Your code does not match what was replaced. What is your 'jsx' referring to ? I don't know what else you say you changed.

On the main branch, in the examples directory, ensure you have run npm install then npm build.

Might be a good idea to do the same in the root first.

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.

None yet

3 participants