PRO-6669: Fix native windows (#5155)

* windows paths

* fix windows

* print stuff

* log more stuff

* more logging

* path-normalizing glob functionality

* run quiet

* convert more slashes

* You can't reliably just pass filenames to async import because C:/ is assumed to be a URL. Use pathToFileUrl

* case

* SASS eats URLs. Not file paths. C:/ messes that up.

* doofus

* already quoted

* be more judicious

* wtf

* handle empty nodes properly

* unreleased

* please eslint

* version

* need a better fake module.import to go with our new calls to it

* accept url objects gracefully

* clean never-before-used db id

* more correct emulation of import, syntax cleanup

* oops

* eslint

* try a newer symlink type that might be less of a permissions problem on Windows

* canonical convert

* consistent convert

* run quiet

* normalize on LF, not CR/LF, in Windows nunjucks output

* uploadfs workaround

* fix attachments options for test purposes

* fix watch rebuild (#5159)

* detect the test path under windows (#5161)

* eslint

---------

Co-authored-by: Miro Yovchev <2827783+myovchev@users.noreply.github.com>

Author: boutell

CHANGELOG.md

@@ -21,7 +21,9 @@
 ### Fixes
 
 * Specify the content type when calling back to Astro with JSON to render an area. This is required starting in Astro 4.9.0 and up, otherwise the request is blocked by CSRF protection.
+* Improved support for Node.js on "plain vanilla" Windows, e.g. without WSL. We suggest working with NVM for Windows and Git Bash.
 * Fixes `AposBreadcrumbSwitch` tooltip prop that is supposed to be an object, not a string. Object returned from the shared method `getOperationTooltip`.
+* Empty text nodes are output properly without a warning.
 * Uses `modalData.locale` in `AposI18nLocalize` component. Fixes watcher on `relatedDocTypes` not being properly triggered (uses data and methods for more control instead).
 * Layout area fix when no columns are present.
 

index.js

@@ -12,10 +12,10 @@ const process = require('process');
 const npmResolve = require('resolve');
 const glob = require('./lib/glob.js');
 const moogRequire = require('./lib/moog-require');
+const importFresh = require('./lib/import-fresh');
+const { pathToFileURL } = require('node:url');
 let defaults = require('./defaults.js');
 
-const importFresh = moduleName => import(`${moduleName}?${Date.now()}`);
-
 // ## Top-level options
 //
 // `cluster`
@@ -363,7 +363,7 @@ async function apostrophe(options, telemetry, rootSpan) {
     const reallyLocalPath = self.rootDir + localPath;
 
     if (fs.existsSync(reallyLocalPath)) {
-      local = await self.root.import(reallyLocalPath);
+      local = await self.root.import(pathToFileURL(reallyLocalPath));
     }
 
     // Otherwise making a second apos instance
@@ -395,7 +395,7 @@ async function apostrophe(options, telemetry, rootSpan) {
     const configs = glob(self.localModules + '/**/modules.js', { follow: true });
     for (const config of configs) {
       try {
-        _.merge(self.options.modules, await self.root.import(config));
+        _.merge(self.options.modules, await self.root.import(pathToFileURL(config)));
       } catch (e) {
         console.error(stripIndent`
           When nestedModuleSubdirs is active, any modules.js file beneath:
@@ -513,7 +513,7 @@ async function apostrophe(options, telemetry, rootSpan) {
     checkTestModule();
     // Allow tests to be in test/ or in tests/
     const testDir = path.dirname(m.filename);
-    const moduleDir = testDir.replace(/\/tests?$/, '');
+    const moduleDir = testDir.replace(/\/tests?$/, '').replace(/\\tests?$/, '');
     if (testDir === moduleDir) {
       throw new Error('Test file must be in test/ or tests/ subdirectory of module');
     }
@@ -527,7 +527,7 @@ async function apostrophe(options, telemetry, rootSpan) {
 
     if (!fs.existsSync(testDir + '/node_modules')) {
       fs.mkdirSync(testDir + '/node_modules' + pkgNamespace, { recursive: true });
-      fs.symlinkSync(moduleDir, testDir + '/node_modules/' + pkgName, 'dir');
+      fs.symlinkSync(moduleDir, testDir + '/node_modules/' + pkgName, 'junction');
     }
     // Makes sure we encounter mocha along the way
     // and throws an exception if we don't
@@ -721,7 +721,7 @@ async function apostrophe(options, telemetry, rootSpan) {
           if (fs.existsSync(path.resolve(self.localModules, name, 'modules.js'))) {
             return;
           }
-          const submodule = await self.root.import(path.resolve(self.localModules, name, 'index.js'));
+          const submodule = await self.root.import(pathToFileURL(path.resolve(self.localModules, name, 'index.js')));
           if (
             submodule &&
             submodule.options &&
@@ -901,8 +901,19 @@ function getRoot(options) {
   if (root?.filename && root?.require) {
     return {
       filename: root.filename,
-      import: async (id) => root.require(id),
-      require: (id) => root.require(id)
+      async import(id) {
+        // Must accept URL objects
+        id = id.toString();
+        // To accurately simulate ES import, we need to
+        // accept file:// URLs like it can
+        if (id.startsWith('file:')) {
+          id = url.fileURLToPath(id);
+        }
+        return root.require(id);
+      },
+      require(id) {
+        return root.require(id);
+      }
     };
   }
 
@@ -944,8 +955,19 @@ function getRoot(options) {
   const legacyRoot = getLegacyRoot();
   return {
     filename: legacyRoot.filename,
-    import: async (id) => legacyRoot.require(id),
-    require: (id) => legacyRoot.require(id)
+    async import(id) {
+      // Must accept URL objects
+      id = id.toString();
+      // To accuratesly simulate ES import, we need to
+      // accept file:// URLs like it can
+      if (id.startsWith('file:')) {
+        id = url.fileURLToPath(id);
+      }
+      return legacyRoot.require(id);
+    },
+    require(id) {
+      return legacyRoot.require(id);
+    }
   };
 };
 

lib/glob.js

@@ -1,10 +1,12 @@
 const { globSync } = require('glob');
 
 // synchronous glob 10 but with the sorting semantics of glob 8,
-// to ease backwards compatibility in Apostrophe startup logic
+// to ease backwards compatibility in Apostrophe startup logic.
+// Also replaces \ with / for consistency across platforms
 
 module.exports = (pattern, options) => {
-  const result = globSync(pattern, options);
+  pattern = pattern.replaceAll('\\', '//');
+  const result = globSync(pattern, options).map(path => path.replaceAll('\\', '/'));
   if (!options.nosort) {
     result.sort((a, b) => a.localeCompare(b, 'en'));
   }

lib/import-fresh.js

@@ -0,0 +1,9 @@
+module.exports = importFresh;
+
+function importFresh(name) {
+  // Don't bomb on plain Windows
+  if (name.match(/^[A-Za-z]:/)) {
+    name = `file:///${name.replaceAll('\\', '/')}`;
+  }
+  return import(`${name}?${Date.now()}`);
+}

lib/moog-require.js

@@ -5,8 +5,7 @@ const path = require('path');
 const glob = require('./glob.js');
 const resolveFrom = require('resolve-from');
 const regExpQuote = require('regexp-quote');
-
-const importFresh = moduleName => import(`${moduleName}?${Date.now()}`);
+const importFresh = require('./import-fresh');
 
 module.exports = async function(options) {
   const self = require('./moog')(options);

lib/moog.js

@@ -189,7 +189,7 @@ module.exports = function(options) {
         }
         // You can have access to options within a function, if you choose to
         // provide one
-        const properties = ((typeof step[cascade]) === 'function') ? step[cascade](that, options) : step[cascade];
+        const properties = ((typeof step[cascade]) === 'function') ? await step[cascade](that, options) : step[cascade];
         if (properties) {
           const valid = [ 'add', 'remove', 'order', 'group' ];
           if (properties.add) {

modules/@apostrophecms/asset/index.js

@@ -6,7 +6,7 @@ const { stripIndent } = require('common-tags');
 const { createId } = require('@paralleldrive/cuid2');
 const chokidar = require('chokidar');
 const _ = require('lodash');
-const { glob } = require('glob');
+const { glob } = require('./lib/path');
 const globalIcons = require('./lib/globalIcons');
 const {
   checkModulesWebpackConfig,
@@ -939,7 +939,8 @@ module.exports = {
         const pulledChanges = [];
         let change = changes.pop();
         while (change) {
-          pulledChanges.push(change);
+          // Fix windows paths
+          pulledChanges.push(change.replace(/\\/g, '/'));
           change = changes.pop();
         }
         // No changes - should never happen.

modules/@apostrophecms/asset/lib/build/external-module-api.js

@@ -1,7 +1,8 @@
 const fs = require('fs-extra');
 const path = require('node:path');
-const { glob } = require('glob');
+const { glob } = require('../../lib/path');
 const { stripIndent } = require('common-tags');
+const { pathToFileURL } = require('node:url');
 
 // High and Low level public API for external modules.
 module.exports = (self) => {
@@ -653,16 +654,17 @@ function invoke() {
             }
           }
         }
-        const jsFilename = JSON.stringify(component);
+        // We know realPath is a file path at this point
+        const importUrl = JSON.stringify(pathToFileURL(realPath));
         const name = self.getComponentNameByPath(
           component,
           { enumerate: options.enumerateImports === true ? i : false }
         );
         const jsName = JSON.stringify(name);
         const importName = `${name}${options.importSuffix || ''}`;
         const importCode = options.importName === false
-          ? `import ${jsFilename};\n`
-          : `import ${importName} from ${jsFilename};\n`;
+          ? `import ${importUrl};\n`
+          : `import ${importName} from ${importUrl};\n`;
 
         output.importCode += `${importCode}`;
 
@@ -720,6 +722,7 @@ function invoke() {
         let importName = importFrom;
         if (!importIndex.includes(importFrom)) {
           if (importFrom.substring(0, 1) === '~') {
+            // An npm module name
             importName = self.apos.util.slugify(importFrom).replaceAll('-', '');
             output.importCode += `import ${importName}Icon from '${importFrom.substring(1)}';\n`;
           } else {

modules/@apostrophecms/asset/lib/build/internals.js

@@ -1,7 +1,7 @@
 const fs = require('fs-extra');
 const path = require('node:path');
 const util = require('node:util');
-const { glob } = require('glob');
+const { glob } = require('../../lib/path');
 const { getBuildExtensions, fillExtraBundles } = require('./utils');
 
 // Internal build interface.
@@ -127,11 +127,12 @@ module.exports = (self) => {
     // Get the component name from a file path. The `enumerate` option allows
     // to append a number to the component name.
     getComponentNameByPath(componentPath, { enumerate } = {}) {
-      return path
+      const result = path
         .basename(componentPath)
         .replace(/-/g, '_')
         .replace(/\s+/g, '')
         .replace(/\.\w+/, '') + (typeof enumerate === 'number' ? `_${enumerate}` : '');
+      return result;
     },
 
     // Return the reported by the external module during build dev server URL.

modules/@apostrophecms/asset/lib/build/task.js

@@ -5,6 +5,7 @@ const fs = require('fs-extra');
 const { stripIndent } = require('common-tags');
 const webpackModule = require('webpack');
 const { mergeWithCustomize: webpackMerge } = require('webpack-merge');
+const { pathToFileURL } = require('node:url');
 const {
   getBundlesNames,
   writeBundlesImportFiles,
@@ -675,12 +676,13 @@ module.exports = (self) => ({
                   `);
           }
         }
-        const jsFilename = JSON.stringify(component);
+        // We know component is a file path at this point
+        const importUrl = JSON.stringify(pathToFileURL(component));
         const name = getComponentName(component, options, i);
         const jsName = JSON.stringify(name);
         const importName = `${name}${options.importSuffix || ''}`;
         const importCode = `
-              import ${importName} from ${jsFilename};
+              import ${importName} from ${importUrl};
               `;
 
         output.paths.push(component);