loader: implementation cleanup

Co-authored-by: GeoffreyBooth <GeoffreyBooth@users.noreply.github.com>

Author: efekrskl

lib/internal/modules/esm/assert.js

@@ -30,19 +30,13 @@ const formatTypeMap = {
   'commonjs': kImplicitTypeAttribute,
   'json': 'json',
   'module': kImplicitTypeAttribute,
+  'text': 'text',
   'wasm': kImplicitTypeAttribute, // It's unclear whether the HTML spec will require an type attribute or not for Wasm; see https://github.com/WebAssembly/esm-integration/issues/42
 };
 // NOTE: Don't add bytes support yet as it requires Uint8Arrays backed by immutable ArrayBuffers,
 // which V8 does not support yet.
 // see: https://github.com/nodejs/node/pull/62300#issuecomment-4079163816
 
-function getFormatType(format) {
-  if (format === 'text' && getOptionValue('--experimental-import-text')) {
-    return 'text';
-  }
-
-  return formatTypeMap[format];
-}
 
 /**
  * The HTML spec disallows the default type to be explicitly specified
@@ -71,7 +65,13 @@ function validateAttributes(url, format,
       throw new ERR_IMPORT_ATTRIBUTE_UNSUPPORTED(keys[i], importAttributes[keys[i]], url);
     }
   }
-  const validType = getFormatType(format);
+  const validType = formatTypeMap[format];
+
+  if (validType !== undefined &&
+    importAttributes.type === 'text' &&
+    !getOptionValue('--experimental-import-text')) {
+    throw new ERR_IMPORT_ATTRIBUTE_UNSUPPORTED('type', importAttributes.type, url);
+  }
 
   switch (validType) {
     case undefined:
@@ -112,8 +112,7 @@ function handleInvalidType(url, type) {
   validateString(type, 'type');
 
   // `type` might not have been one of the types we understand.
-  if (!ArrayPrototypeIncludes(supportedTypeAttributes, type) &&
-      !(type === 'text' && getOptionValue('--experimental-import-text'))) {
+  if (!ArrayPrototypeIncludes(supportedTypeAttributes, type)) {
     throw new ERR_IMPORT_ATTRIBUTE_UNSUPPORTED('type', type, url);
   }
 

lib/internal/modules/esm/get_format.js

@@ -236,12 +236,21 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
   throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath);
 }
 
+function isExperimentalTextImport(importAttributes) {
+  return getOptionValue('--experimental-import-text') &&
+    importAttributes?.type === 'text';
+}
+
 /**
  * @param {URL} url
- * @param {{parentURL: string}} context
+ * @param {{parentURL: string, importAttributes?: string}} context
  * @returns {Promise<string> | string | undefined} only works when enabled
  */
 function defaultGetFormatWithoutErrors(url, context) {
+  if (isExperimentalTextImport(context?.importAttributes)) {
+    return 'text';
+  }
+
   const protocol = url.protocol;
   if (!ObjectPrototypeHasOwnProperty(protocolHandlers, protocol)) {
     return null;
@@ -255,6 +264,10 @@ function defaultGetFormatWithoutErrors(url, context) {
  * @returns {Promise<string> | string | undefined} only works when enabled
  */
 function defaultGetFormat(url, context) {
+  if (isExperimentalTextImport(context?.importAttributes)) {
+    return 'text';
+  }
+
   const protocol = url.protocol;
   if (!ObjectPrototypeHasOwnProperty(protocolHandlers, protocol)) {
     return null;

lib/internal/modules/esm/load.js

@@ -6,7 +6,6 @@ const {
 const {
   kEmptyObject,
 } = require('internal/util');
-const { getOptionValue } = require('internal/options');
 
 const { defaultGetFormat } = require('internal/modules/esm/get_format');
 const { validateAttributes, emitImportAssertionWarning } = require('internal/modules/esm/assert');
@@ -49,11 +48,6 @@ function getSourceSync(url, context) {
   return { __proto__: null, responseURL, source };
 }
 
-function isExperimentalTextImport(importAttributes) {
-  return getOptionValue('--experimental-import-text') &&
-    importAttributes?.type === 'text';
-}
-
 /**
  * Node.js default load hook.
  * @param {string} url
@@ -95,12 +89,8 @@ function defaultLoad(url, context = kEmptyObject) {
     }
 
     if (format == null) {
-      if (isExperimentalTextImport(importAttributes)) {
-        format = 'text';
-      }
-
       // Now that we have the source for the module, run `defaultGetFormat` to detect its format.
-      format ??= defaultGetFormat(urlInstance, context);
+      format = defaultGetFormat(urlInstance, context);
 
       if (format === 'commonjs') {
         // For backward compatibility reasons, we need to discard the source in
@@ -163,10 +153,6 @@ function defaultLoadSync(url, context = kEmptyObject) {
       context = { __proto__: context, source };
     }
 
-    if (format == null && isExperimentalTextImport(importAttributes)) {
-      format = 'text';
-    }
-
     // Now that we have the source for the module, run `defaultGetFormat` to detect its format.
     format ??= defaultGetFormat(urlInstance, context);
 

lib/internal/modules/esm/resolve.js

@@ -943,7 +943,6 @@ function throwIfInvalidParentURL(parentURL) {
  */
 function defaultResolve(specifier, context = {}) {
   let { parentURL, conditions } = context;
-  const { importAttributes } = context;
   throwIfInvalidParentURL(parentURL);
 
   let parsedParentURL;
@@ -1006,18 +1005,12 @@ function defaultResolve(specifier, context = {}) {
     throw error;
   }
 
-  let format = defaultGetFormatWithoutErrors(url, context);
-  if (getOptionValue('--experimental-import-text') &&
-      importAttributes?.type === 'text') {
-    format = 'text';
-  }
-
   return {
     __proto__: null,
     // Do NOT cast `url` to a string: that will work even when there are real
     // problems, silencing them
     url: url.href,
-    format,
+    format: defaultGetFormatWithoutErrors(url, context),
   };
 }