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(admin): loadConfig does not allow serve to be truly configurable #11238

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

adevinwild
Copy link
Contributor

Related to #11237

Solution that PR introduce

  • serve is always true allowing to always start the Admin UI with the new logic in the develop.js command file in @medusajs/medusa
  • loadConfig in the @medusajs/admin is no longer overriding the serve function to false, allowing to fix the current bug we have in v1.20.11 where the Admin UI cannot be launched at all

Quick preview of the new behavior on a project

In this preview, I try with the different values and how it behaves with the fix.

  • No serve option -> Default to true; Trigger the admin develop
  • serve set to false -> Ignore the admin develop command
  • serve set to true -> Trigger the admin develop
loadConfig.mp4

- `serve` is always `true` allowing to always start the Admin UI with the new logic in the `develop.js` command file in @medusajs/medusa
- `loadConfig` in the @medusajs/admin is no longer overriding the `serve` function to false, allowing to fix the current bug we have in 1.20.11 where the Admin UI cannot be launched at all
@adevinwild adevinwild requested a review from a team as a code owner January 31, 2025 05:43
Copy link

changeset-bot bot commented Jan 31, 2025

🦋 Changeset detected

Latest commit: d42c10e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@medusajs/admin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Jan 31, 2025

Someone is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

@@ -35,7 +35,6 @@ export const loadConfig = (isDev?: boolean): PluginOptions | null => {
if (isDev) {
config = {
...config,
serve: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We remove the forced false value here to make sure it's configurable as explained in the docs

@@ -45,7 +44,7 @@ export const loadConfig = (isDev?: boolean): PluginOptions | null => {
const options = (plugin as { options: PluginOptions }).options ?? {}

config = {
serve: isDev ? false : options.serve ?? config.serve,
serve: options.serve ?? config.serve,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same goes here, to make sure it's fully configurable

Copy link
Contributor

@kasperkristensen kasperkristensen left a comment

Choose a reason for hiding this comment

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

Thanks @adevinwild LGTM

@kodiakhq kodiakhq bot merged commit a34ddda into medusajs:v1.x Feb 7, 2025
19 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants