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.

Sign up for GitHub

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

Jump to bottom

New vite based application runtime #1881

Merged
merged 107 commits into from
Apr 20, 2023
Merged

New vite based application runtime #1881

merged 107 commits into from
Apr 20, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Apr 12, 2023 •
edited
Loading

Use vite to build the Toolpad applications:

  • Allows using installed node modules in Toolpad applications
  • Faster production runtime that doesn't pull in every icon and every MUI component
  • Allows running react in dev mode in the canvas to allow for richer error messages
  • Added behind a --viteRuntime CLI flag to allow canary testing

Additional fixes:

  • canvas overlay disappeared when the canvas iframe reloaded, added a fix for that
  • store test results in CI for debuggability
  • fix /preview/pages not redirecting to the first page
  • Remove next/image usage
  • Align some logs to the format throughout the app
  • dragNewComponentToAppCanvas was exiting before the drag was completed, resulting in failures if the test wasn't waiting on anything afterwards
  • Recreate yarn.lock

I can open a separate PR for those if you want, but most are needed for this PR as well

Closes #1008
Closes #1516

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Apr 12, 2023
@Janpot Janpot mentioned this pull request Apr 12, 2023
Merged
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 12, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 12, 2023
@Janpot Janpot changed the title Allow using installed node modules in Toolpad applications Allow using installed node modules in custom Toolpad components Apr 12, 2023
@Janpot Janpot marked this pull request as ready for review April 19, 2023 16:15
@Janpot Janpot mentioned this pull request Apr 19, 2023
Merged
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 19, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 19, 2023
@Janpot Janpot requested a review from apedroferreira April 19, 2023 19:44
apedroferreira
apedroferreira approved these changes Apr 19, 2023
View reviewed changes
test/models/ToolpadEditor.ts Outdated
@@ -144,23 +144,25 @@ export class ToolpadEditor {
await this.page.mouse.up();
}

componentCatalogItem(name: string): Locator {
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe getComponentCatalogItem?

Copy link
Member Author

@Janpot Janpot Apr 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I tried to be consistent with hierarchyItem below. I'm renaming both

test/models/ToolpadEditor.ts

this.dragToAppCanvas(sourceLocator, moveTargetX, moveTargetY);
await this.dragToAppCanvas(sourceLocator, moveTargetX, moveTargetY);
Copy link
Member

Choose a reason for hiding this comment

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

👍

@apedroferreira
Copy link
Member

apedroferreira commented Apr 19, 2023 •
edited
Loading

Great feature!
Hopefully Vite makes the development experience a bit faster too since it's more important in the new direction.

apedroferreira
apedroferreira reviewed Apr 19, 2023
View reviewed changes
Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

Wait, I had checked only the code but tried to run it and got:

Screenshot 2023-04-19 at 23 13 29

@apedroferreira apedroferreira self-requested a review April 19, 2023 22:16
@Janpot
Copy link
Member Author

Janpot commented Apr 20, 2023

Wait, I had checked only the code but tried to run it and got:

Currently, to try out the new functionality you'll have to run the toolpad cli with --viteRuntime flag. Looks like I had a problem with the old mode. Fixed and running the tests in old mode to make sure it still works

apedroferreira
apedroferreira approved these changes Apr 20, 2023
View reviewed changes
Copy link
Member

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

yarn dev works now!
Also I've tried a custom component using recharts and it worked great.

Screenshot 2023-04-20 at 09 49 17

@Janpot
Copy link
Member Author

Janpot commented Apr 20, 2023

Yes, I've added an integration test for recharts.

@Janpot Janpot enabled auto-merge (squash) April 20, 2023 08:59
@Janpot Janpot merged commit 9df3b90 into master Apr 20, 2023
@Janpot Janpot deleted the vite-experiment branch April 20, 2023 09:15
This was referenced Jun 12, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apedroferreira apedroferreira apedroferreira approved these changes

Assignees
No one assigned
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Debugging react code in production from editor Decrease bundle size
2 participants
@Janpot @apedroferreira