From a89d4cf348f7edc0a52b8ab9aacf96f2de939de4 Mon Sep 17 00:00:00 2001
From: Kai Vandivier <49666798+KaiVandivier@users.noreply.github.com>
Date: Mon, 3 Jun 2024 20:30:29 +0200
Subject: [PATCH] fix: don't start plugins for apps without a plugin entrypoint
 (#850)

* fix: don't start plugins for apps without a plugin entrypoint

* fix: don't start app when there's just a plugin entrypoint

* fix: implement feedback

* fix: simplify boolean logic

* fix: improve message again
---
 cli/src/commands/start.js             | 34 ++++++++++++++++++++-------
 cli/src/lib/generateManifests.js      | 23 +-----------------
 cli/src/lib/getOriginalEntrypoints.js | 26 ++++++++++++++++++++
 3 files changed, 52 insertions(+), 31 deletions(-)
 create mode 100644 cli/src/lib/getOriginalEntrypoints.js

diff --git a/cli/src/commands/start.js b/cli/src/commands/start.js
index 6370b7651..3924f5b41 100644
--- a/cli/src/commands/start.js
+++ b/cli/src/commands/start.js
@@ -3,6 +3,7 @@ const detectPort = require('detect-port')
 const { compile } = require('../lib/compiler')
 const exitOnCatch = require('../lib/exitOnCatch')
 const generateManifests = require('../lib/generateManifests')
+const { getOriginalEntrypoints } = require('../lib/getOriginalEntrypoints')
 const i18n = require('../lib/i18n')
 const loadEnvFiles = require('../lib/loadEnvFiles')
 const parseConfig = require('../lib/parseConfig')
@@ -129,13 +130,28 @@ const handler = async ({
             reporter.info('Starting development server...')
 
             // start app and/or plugin, depending on flags
-            const shouldStartBoth =
-                (!shouldStartOnlyApp && !shouldStartOnlyPlugin) ||
-                // it would be weird to use both flags, but start both if so
-                (shouldStartOnlyApp && shouldStartOnlyPlugin)
-            const startPromises = []
 
-            if (shouldStartBoth || shouldStartOnlyApp) {
+            // entryPoints.app is populated by default -- get the app's raw
+            // entry points from the d2.config.js file to tell if there is an
+            // app entrypoint configured (and fall back to the whatever we do
+            // have available)
+            const entryPoints =
+                getOriginalEntrypoints(paths) || config.entryPoints
+
+            const noFlagsPassed = !shouldStartOnlyApp && !shouldStartOnlyPlugin
+            const shouldStartApp =
+                entryPoints.app && (noFlagsPassed || shouldStartOnlyApp)
+            const shouldStartPlugin =
+                entryPoints.plugin && (noFlagsPassed || shouldStartOnlyPlugin)
+
+            if (!shouldStartApp && !shouldStartPlugin) {
+                throw new Error(
+                    'The requested app/plugin is not configured to start. Check the flags passed to the start script and the entrypoints configured in d2.config.js, then try again.'
+                )
+            }
+
+            const startPromises = []
+            if (shouldStartApp) {
                 reporter.print(
                     `The app ${chalk.bold(
                         config.name
@@ -145,12 +161,12 @@ const handler = async ({
                 startPromises.push(shell.start({ port: newPort }))
             }
 
-            if (shouldStartBoth || shouldStartOnlyPlugin) {
+            if (shouldStartPlugin) {
+                const pluginPort = shouldStartApp ? newPort + 1 : newPort
                 reporter.print(
-                    `The plugin is now available on port ${newPort} at /${paths.pluginLaunchPath}`
+                    `The plugin is now available on port ${pluginPort} at /${paths.pluginLaunchPath}`
                 )
                 reporter.print('')
-                const pluginPort = shouldStartBoth ? newPort + 1 : newPort
                 startPromises.push(plugin.start({ port: pluginPort }))
             }
 
diff --git a/cli/src/lib/generateManifests.js b/cli/src/lib/generateManifests.js
index 6ba6db624..21287b25f 100644
--- a/cli/src/lib/generateManifests.js
+++ b/cli/src/lib/generateManifests.js
@@ -1,28 +1,7 @@
 const { reporter, chalk } = require('@dhis2/cli-helpers-engine')
 const fs = require('fs-extra')
+const { getOriginalEntrypoints } = require('./getOriginalEntrypoints')
 
-/**
- * Gets the original `entrypoints` property in d2.config.js
- * without applying defaults. Used to detect if there is actually
- * supposed to be an app entrypoint for this... app. Temporary until
- * the build process is redesigned to allow building plugins without
- * apps (LIBS-479)
- */
-const getOriginalEntrypoints = (paths) => {
-    try {
-        if (fs.existsSync(paths.config)) {
-            reporter.debug('Loading d2 config at', paths.config)
-            // NB: this import can be confounded by previous object mutations
-            const originalConfig = require(paths.config)
-            reporter.debug('loaded', originalConfig)
-            return originalConfig.entryPoints // may be undefined
-        }
-    } catch (e) {
-        reporter.error('Failed to load d2 config!')
-        reporter.error(e)
-        process.exit(1)
-    }
-}
 const parseCustomAuthorities = (authorities) => {
     if (!authorities) {
         return undefined
diff --git a/cli/src/lib/getOriginalEntrypoints.js b/cli/src/lib/getOriginalEntrypoints.js
new file mode 100644
index 000000000..5efa0a48d
--- /dev/null
+++ b/cli/src/lib/getOriginalEntrypoints.js
@@ -0,0 +1,26 @@
+const { reporter } = require('@dhis2/cli-helpers-engine')
+const fs = require('fs-extra')
+
+/**
+ * Gets the original `entrypoints` property in d2.config.js
+ * without applying defaults. Used to detect if there is actually
+ * supposed to be an app entrypoint for this... app. Temporary until
+ * the build process is redesigned to allow building plugins without
+ * apps (LIBS-479)
+ */
+const getOriginalEntrypoints = (paths) => {
+    try {
+        if (fs.existsSync(paths.config)) {
+            reporter.debug('Loading d2 config at', paths.config)
+            // NB: this import can be confounded by previous object mutations
+            const originalConfig = require(paths.config)
+            reporter.debug('loaded', originalConfig)
+            return originalConfig.entryPoints // may be undefined
+        }
+    } catch (e) {
+        reporter.error('Failed to load d2 config!')
+        reporter.error(e)
+        process.exit(1)
+    }
+}
+exports.getOriginalEntrypoints = getOriginalEntrypoints