refactor and polish

vikejs · Jun 5, 2024 · 8bdf055 · 8bdf055
1 parent 7b30d71
commit 8bdf055
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 57 deletions.
diff --git a/vike/node/plugin/plugins/importUserCode/v1-design/assertExtensions.ts b/vike/node/plugin/plugins/importUserCode/v1-design/assertExtensions.ts
@@ -1,70 +1,85 @@
-export { assertExtensionsPeerDependencies }
 export { assertExtensionsConventions }
+export { assertExtensionsPeerDependencies }
 
 import pc from '@brillout/picocolors'
 import { isObjectOfStrings } from '../../../../../utils/isObjectOfStrings.js'
 import { PROJECT_VERSION, assert, assertUsage, assertWarning, findPackageJson } from '../../../utils.js'
 import { getConfigValueInterfaceFile, type InterfaceFile } from './getVikeConfig.js'
 import path from 'path'
 import semver from 'semver'
-import type { ConfigFile } from './getVikeConfig/loadFileAtConfigTime.js'
 
-function assertExtensionsConventions(extendsConfig: ConfigFile, interfaceFile: InterfaceFile) {
-  const alreadyMigrated = [
-    'vike-react',
-    'vike-react-query',
-    'vike-react-zustand',
-    'vike-vue',
-    'vike-pinia',
-    'vike-solid'
-  ]
-  const { importPathAbsolute } = extendsConfig.filePath
+function assertExtensionsConventions(interfaceFile: InterfaceFile): void {
+  assertExtensionName(interfaceFile)
+  assertConfigExportPath(interfaceFile)
+}
+function assertConfigExportPath(interfaceFile: InterfaceFile): void {
+  const { importPathAbsolute } = interfaceFile.filePath
   assert(importPathAbsolute)
-  const extensionName = importPathAbsolute
-    .split('/')
-    .slice(0, importPathAbsolute.startsWith('@') ? 2 : 1)
-    .join('/')
-  const errMsg = alreadyMigrated.includes(extensionName)
-    ? `You're using a deprecated version of the Vike extension ${extensionName}, update ${extensionName} to its latest version.`
-    : `The config of the Vike extension ${extensionName} should define the ${pc.cyan('name')} setting.`
-  const extensionNameValue = getConfigValueInterfaceFile(interfaceFile, 'name')
-  if (alreadyMigrated) {
-    // Eventually remove (always use assertUsage())
-    assertWarning(extensionNameValue, errMsg, { onlyOnce: true })
-  } else {
-    assertUsage(extensionNameValue, errMsg)
-  }
+  const name = getConfigNameValue(interfaceFile)
+  assert(name) // already asserted in assertExtensionName()
+  const importPathAbsoluteExpected = `${name}/config`
+  assertWarning(
+    importPathAbsolute === importPathAbsoluteExpected,
+    `The Vike configuration of ${pc.bold(name)} is exported at ${pc.bold(
+      importPathAbsolute
+    )} but it should be exported at ${pc.bold(importPathAbsoluteExpected)} instead.`,
+    { onlyOnce: true }
+  )
+}
+function assertExtensionName(interfaceFile: InterfaceFile): void {
+  let nameDeduced: string
   {
-    const { filePathToShowToUserResolved } = interfaceFile.filePath
-    assert(filePathToShowToUserResolved)
-    const errPrefix = `The setting ${pc.bold('name')} defined at ${filePathToShowToUserResolved}`
-    assertUsage(typeof extensionNameValue === 'string', `${errPrefix} should be a string.`)
-    assertWarning(
-      extensionNameValue === extensionName,
-      `${errPrefix} is ${pc.bold(extensionNameValue)} but it should be ${pc.bold(extensionName)} instead.`,
-      { onlyOnce: true }
-    )
+    const { importPathAbsolute } = interfaceFile.filePath
+    assert(importPathAbsolute)
+    nameDeduced = importPathAbsolute
+      .split('/')
+      .slice(0, importPathAbsolute.startsWith('@') ? 2 : 1)
+      .join('/')
   }
-  {
-    const importPathAbsoluteExpected = `${extensionName}/config`
+
+  const name = getConfigNameValue(interfaceFile)
+  const filePathToShowToUser = getFilePathToShowToUser(interfaceFile)
+  if (name) {
     assertWarning(
-      importPathAbsolute === importPathAbsoluteExpected,
-      `The Vike configuration of ${extensionName} is exported at ${pc.bold(
-        importPathAbsolute
-      )} but it should be exported at ${pc.bold(importPathAbsoluteExpected)} instead.`,
+      name === nameDeduced,
+      `The setting ${pc.bold('name')} defined at ${filePathToShowToUser} is ${pc.bold(name)} but it should be ${pc.bold(
+        nameDeduced
+      )} instead (the name of the npm package).`,
       { onlyOnce: true }
     )
+  } else {
+    if (
+      // Let's eventually remove this
+      [
+        'vike-react',
+        'vike-react-query',
+        'vike-react-zustand',
+        'vike-vue',
+        'vike-vue-query',
+        'vike-vue-pinia',
+        'vike-pinia',
+        'vike-solid'
+      ].includes(nameDeduced)
+    ) {
+      assertUsage(false, `Update ${nameDeduced} to its latest version.`)
+    } else {
+      assertUsage(
+        false,
+        `The setting ${pc.bold('name')} is missing: it should be set by the config ${filePathToShowToUser} of ${pc.bold(
+          nameDeduced
+        )}.`
+      )
+    }
   }
 }
 
-function assertExtensionsPeerDependencies(interfaceFilesRelevantList: InterfaceFile[]) {
+function assertExtensionsPeerDependencies(interfaceFilesRelevantList: InterfaceFile[]): void {
   // Get installed extensions
   const extensions: Record<string, string> = {}
   interfaceFilesRelevantList.forEach((interfaceFile) => {
     const name = getConfigNameValue(interfaceFile)
     if (name) {
-      const extensionConfigFilePath = interfaceFile.filePath.filePathAbsoluteFilesystem
-      const version = getExtensionVersion(extensionConfigFilePath, name)
+      const version = getExtensionVersion(name, interfaceFile)
       extensions[name] = version
     }
   })
@@ -74,10 +89,15 @@ function assertExtensionsPeerDependencies(interfaceFilesRelevantList: InterfaceF
     const require = getConfigRequireValue(interfaceFile)
     if (!require) return
     const name = getConfigNameValue(interfaceFile)
-    // TODO: unify assertUsage()?
-    assertUsage(name, `Setting ${pc.bold('name')} is required when using setting ${pc.bold('require')}.`)
+    const filePathToShowToUser = getFilePathToShowToUser(interfaceFile)
+    assertUsage(
+      name,
+      `Setting ${pc.bold('name')} is required for being able to use setting ${pc.bold(
+        'require'
+      )} in ${filePathToShowToUser}.`
+    )
     Object.entries(require).forEach(([reqName, reqVersion]) => {
-      const errBase = `The Vike extension ${pc.bold(name)} requires ${pc.bold(reqName)}`
+      const errBase = `${pc.bold(name)} requires ${pc.bold(reqName)}`
       if (reqName === 'vike') {
         assertUsage(
           semver.satisfies(PROJECT_VERSION, reqVersion),
@@ -109,32 +129,46 @@ function getConfigRequireValue(interfaceFile: InterfaceFile): null | Record<stri
   )
   return require
 }
+
 function getConfigNameValue(interfaceFile: InterfaceFile): null | string {
   const name = getConfigValueInterfaceFile(interfaceFile, 'name')
   if (!name) return null
-  // TODO: move assertUsage() here
-  assert(typeof name === 'string')
+  const filePathToShowToUser = getFilePathToShowToUser(interfaceFile)
+  assertUsage(
+    typeof name === 'string',
+    `The setting ${pc.bold('name')} defined at ${filePathToShowToUser} should be a string.`
+  )
   return name
 }
 
 // We use a forever cache: users need to restart the dev server anyways when touching node_modules/**/* (I presume Vite doesn't pick up node_modules/**/* changes).
 const extensionsVersion: Record<string, string> = {}
-function getExtensionVersion(extensionConfigFilePath: string, name: string): string {
+function getExtensionVersion(name: string, interfaceFile: InterfaceFile): string {
   if (!extensionsVersion[name]) {
+    const extensionConfigFilePath = interfaceFile.filePath.filePathAbsoluteFilesystem
     const found = findPackageJson(path.posix.dirname(extensionConfigFilePath))
     assert(found)
     const { packageJson, packageJsonPath } = found
-    assertUsage(
-      packageJson.name === name,
-      `The setting ${pc.bold('name')} set by ${extensionConfigFilePath} is ${pc.bold(
+    const filePathToShowToUser = getFilePathToShowToUser(interfaceFile)
+    const nameExpected = packageJson.name
+    assertWarning(
+      name === nameExpected,
+      `The setting ${pc.bold('name')} defined at ${filePathToShowToUser} is ${pc.bold(
         JSON.stringify(name)
-      )} but it should be equal to ${packageJsonPath}${pc.dim('#')}${pc.bold('name')} which is ${pc.bold(
-        JSON.stringify(packageJson.name)
-      )}`
+      )} but it should be equal to ${pc.bold(JSON.stringify(nameExpected))} (the value of ${packageJsonPath}${pc.dim(
+        '#'
+      )}${pc.bold('name')})`,
+      { onlyOnce: true }
     )
     const { version } = packageJson
     assert(typeof version === 'string')
     extensionsVersion[name] = version
   }
   return extensionsVersion[name]!
 }
+
+function getFilePathToShowToUser(interfaceFile: InterfaceFile): string {
+  const { filePathToShowToUserResolved } = interfaceFile.filePath
+  assert(filePathToShowToUserResolved)
+  return filePathToShowToUserResolved
+}
diff --git a/vike/node/plugin/plugins/importUserCode/v1-design/getVikeConfig.ts b/vike/node/plugin/plugins/importUserCode/v1-design/getVikeConfig.ts
@@ -236,7 +236,7 @@ async function loadInterfaceFiles(
         ```
         */
         const interfaceFile = getInterfaceFileFromConfigFile(extendsConfig, true, locationId)
-        assertExtensionsConventions(extendsConfig, interfaceFile)
+        assertExtensionsConventions(interfaceFile)
         interfaceFilesByLocationId[locationId]!.push(interfaceFile)
       })
     }),