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

Allow integrating toolpad applications in a custom server #2747

Merged
merged 16 commits into from
Oct 6, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Oct 2, 2023

Export a unstable_createHandler which can be used to integrated in a custom server. See example.

Can be imported as ESM, should also work under commonjs. In any case it can always dynamically be imported as ESM under commonjs.

Changes include:

  • make serverside code ESM compatible
  • refactor the intiialization of options so that it can be shared between cli and custom server
  • export unstable_createHandler
  • remove projectRoot. (dead code)
  • disable the preview header in custom server (for now at least). It's not ideal because:
    • It contains the theme switcher for the editor. Which theme should it choose? Does it even make sense in the first place to have the editor theme in the preview window?
    • contains a button to open the editor, while there is nothing to open (yet)

Follow up work includes:

  • Allow editing apps running under custom server

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Oct 2, 2023
@Janpot Janpot marked this pull request as ready for review October 2, 2023 15:49
@Janpot Janpot requested a review from a team October 2, 2023 15:49
@Janpot Janpot changed the title Make sure node.js code can work both under commonjs and ESM Export unstable_createHandler from @mui/toolpad Oct 4, 2023
@Janpot Janpot changed the title Export unstable_createHandler from @mui/toolpad Allow integrating toolpad applications in a custom server Oct 4, 2023
@Janpot Janpot requested a review from a team October 4, 2023 08:55
@Janpot
Copy link
Member Author

Janpot commented Oct 4, 2023

Putting up for rereview. I turned this PR into exporting the toolpad handler as it turned out there were more things that needed to be done than was initially proposed

@Janpot Janpot added the new feature New feature or request label Oct 4, 2023
@apedroferreira

This comment was marked as resolved.

dev: process.env.NODE_ENV === 'development',
base: appBase,
});
app.use('/my-app', handler.handler);
Copy link
Member

Choose a reason for hiding this comment

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

So in the example /my-app is the path that the whole Toolpad runs under including the editor in dev mode, and /my-app is the base path where the production app runs?
If so, should they be different to be more obvious?

Copy link
Member Author

@Janpot Janpot Oct 5, 2023

Choose a reason for hiding this comment

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

including the editor in dev mode

Not the editor, only the application. For editing I'm envisioning the following:

  • you start your custom server with the Toolpad handler in dev mode.
  • you run yarn toolpad --connect http://localhost:3000/my-app
  • then follow the link that's printed to open the editor where the canvas is actually pointing to your custom server

Only the end user application runs in your server

Copy link
Member

Choose a reason for hiding this comment

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

I see, that sounds cool!
Are there cases where the first argument for app.use and appBase would not be the same, though? Or could they share the same variable?
Anyway that's not important at all so I'm approving already.

Copy link
Member Author

@Janpot Janpot Oct 5, 2023

Choose a reason for hiding this comment

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

it's an example. for demonstration purposes is it a bit clearer.

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.

Just noticed a couple of things maybe worth mentioning, everything else looks good!

@Janpot
Copy link
Member Author

Janpot commented Oct 5, 2023

I'm getting an error for a missing exports property in package.json when trying to run the custom server example.

How are you running it? The following should work:

  • add examples/custom-server to the package.json workspaces field
  • run yarn && yarn dev --ignore custom-server
  • run yarn workspace custom-server dev

@Janpot Janpot merged commit 419a804 into mui:master Oct 6, 2023
11 checks passed
@Janpot Janpot deleted the fix-cjs branch October 6, 2023 08:06
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 new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants