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

[core] Add canvas mode that doesn't rely on vite #3171

Merged
merged 40 commits into from
Feb 9, 2024

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Feb 6, 2024

First batch of changes that renders the canvas in the same react tree as the editor instead of loading a page in an iframe. Currently behind a flag EXPERIMENTAL_INLINE_CANVAS because more work is required to make it the main rendering mode. But aiming to already merge this as to avoid accumulating many big architectural changes in one PR.

Now the editor is hosted under the same path as the app:

  • /{basepath}/app/pages/... => application
  • /{basepath}/editor/pages/... => editor (new)

things that are now fixed

  • Both editor and app are now built in the same vite config, meaning we won't get clashing deps optimization anymore
  • The canvas is rendered directly in the react tree of the editor, so now pages load immediately in the editor
  • typescript works when editor is in dev mode (and it's always in dev mode now during development)

add to the .env file to play with this:

# .env
EXPERIMENTAL_INLINE_CANVAS=1

To do (in follow up):

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Feb 6, 2024
@Janpot Janpot changed the title [core] Remove canvas vite [core] Add canvas mode that doesn't rely on vite Feb 7, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2024
@Janpot Janpot marked this pull request as ready for review February 8, 2024 19:20
@Janpot Janpot requested a review from a team February 8, 2024 19:21
@Janpot Janpot merged commit d698ed1 into mui:master Feb 9, 2024
11 checks passed
@Janpot Janpot deleted the next-support branch February 9, 2024 10:24
@apedroferreira
Copy link
Member

I'm trying this now and it seems like when running pnpm dev the root page (e.g.: http://localhost:3000) doesn't default to the editor? Not sure if that's intentional.

@Janpot
Copy link
Member Author

Janpot commented Mar 1, 2024

When I do

pnpm dev

and then

pnpm toolpad dev test/integration/backend-basic/fixture/toolpad --dev

it opens http://localhost:3000/prod/editor/ for me

I have

# .env
EXPERIMENTAL_INLINE_CANVAS=1

@Janpot
Copy link
Member Author

Janpot commented Mar 1, 2024

One problem I'm experiencing in this mode is with

pnpm toolpad dev test/visual/components/fixture/toolpad --dev

and then opening http://localhost:3000/prod/editor/app/pages/components
Any component I drag on the page locks the app up completely

@apedroferreira
Copy link
Member

When I do

pnpm dev

and then

pnpm toolpad dev test/integration/backend-basic/fixture/toolpad --dev

it opens http://localhost:3000/prod/editor/ for me

I have

# .env
EXPERIMENTAL_INLINE_CANVAS=1

Weird, it's working fine for me when I tried now. Maybe something was wrong on my local environment yesterday...

One problem I'm experiencing in this mode is with

pnpm toolpad dev test/visual/components/fixture/toolpad --dev

and then opening http://localhost:3000/prod/editor/app/pages/components Any component I drag on the page locks the app up completely

Just tried it and it seems to work fine for me?

@Janpot
Copy link
Member Author

Janpot commented Mar 1, 2024

Just tried it and it seems to work fine for me?

Make sure to have EXPERIMENTAL_INLINE_CANVAS=1

@apedroferreira
Copy link
Member

Just tried it and it seems to work fine for me?

Make sure to have EXPERIMENTAL_INLINE_CANVAS=1

Ah I had forgotten to set the environment variable in the test fixture. Can confirm the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants