Make it a non-breaking change

parcel-bundler · Dec 29, 2024 · f682727 · f682727
1 parent 14f7bee
commit f682727
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 22 deletions.
diff --git a/packages/configs/default/index.json b/packages/configs/default/index.json
@@ -8,8 +8,14 @@
       "@parcel/transformer-worklet",
       "..."
     ],
-    "*.mdx": ["@parcel/transformer-js"],
-    "*.{js,mjs,jsm,jsx,es6,cjs,ts,tsx,mdx}": [
+    "*.mdx": [
+      // For backward compatibility, include the old transformer
+      // so it is used if already installed in the project.
+      // Otherwise, the JS transformer will handle MDX.
+      "@parcel/transformer-mdx",
+      "@parcel/transformer-js"
+    ],
+    "*.{js,mjs,jsm,jsx,es6,cjs,ts,tsx}": [
       "@parcel/transformer-babel",
       "@parcel/transformer-js",
       "@parcel/transformer-react-refresh-wrap"

diff --git a/packages/configs/default/package.json b/packages/configs/default/package.json
@@ -74,6 +74,9 @@
     "@parcel/transformer-xml": "2.13.3",
     "@parcel/transformer-yaml": "2.13.3"
   },
+  "optionalParcelDependencies": [
+    "@parcel/transformer-mdx"
+  ],
   "peerDependencies": {
     "@parcel/core": "^2.13.3"
   }

diff --git a/packages/configs/default/test/config.test.js b/packages/configs/default/test/config.test.js
@@ -2,8 +2,9 @@
 
 import assert from 'assert';
 
-import config from '../';
 import packageJson from '../package.json';
+import json5 from 'json5';
+import fs from 'fs';
 
 describe('@parcel/config-default', () => {
   let packageJsonDependencyNames: Set<string>;
@@ -13,7 +14,11 @@ describe('@parcel/config-default', () => {
     packageJsonDependencyNames = new Set([
       ...Object.keys(packageJson.dependencies || {}),
       ...Object.keys(packageJson.parcelDependencies || {}),
+      ...packageJson.optionalParcelDependencies,
     ]);
+    let config = json5.parse(
+      fs.readFileSync(__dirname + '/../index.json', 'utf8'),
+    );
     configPackageReferences = collectConfigPackageReferences(config);
   });
 

diff --git a/packages/core/core/src/ParcelConfig.js b/packages/core/core/src/ParcelConfig.js
@@ -28,6 +28,7 @@ import ThrowableDiagnostic, {
   generateJSONCodeHighlights,
 } from '@parcel/diagnostic';
 import json5 from 'json5';
+import nullthrows from 'nullthrows';
 
 import {globToRegex} from '@parcel/utils';
 import {basename} from 'path';
@@ -121,7 +122,7 @@ export default class ParcelConfig {
     version: Semver,
     resolveFrom: ProjectPath,
     range: ?SemverRange,
-  |}> {
+  |} | null> {
     let plugin = this.pluginCache.get(node.packageName);
     if (plugin) {
       return plugin;
@@ -138,8 +139,11 @@ export default class ParcelConfig {
     return plugin;
   }
 
-  async loadPlugin<T>(node: ParcelPluginNode): Promise<LoadedPlugin<T>> {
+  async loadPlugin<T>(node: ParcelPluginNode): Promise<LoadedPlugin<T> | null> {
     let plugin = await this._loadPlugin(node);
+    if (!plugin) {
+      return null;
+    }
     return {
       ...plugin,
       name: node.packageName,
@@ -151,10 +155,12 @@ export default class ParcelConfig {
     this.pluginCache.delete(packageName);
   }
 
-  loadPlugins<T>(
+  async loadPlugins<T>(
     plugins: PureParcelConfigPipeline,
   ): Promise<Array<LoadedPlugin<T>>> {
-    return Promise.all(plugins.map(p => this.loadPlugin<T>(p)));
+    return (await Promise.all(plugins.map(p => this.loadPlugin<T>(p)))).filter(
+      Boolean,
+    );
   }
 
   async getResolvers(): Promise<Array<LoadedPlugin<Resolver<mixed>>>> {
@@ -228,7 +234,7 @@ export default class ParcelConfig {
       );
     }
 
-    return this.loadPlugin<Bundler<mixed>>(this.bundler);
+    return nullthrows(await this.loadPlugin<Bundler<mixed>>(this.bundler));
   }
 
   async getNamers(): Promise<Array<LoadedPlugin<Namer<mixed>>>> {
@@ -265,7 +271,7 @@ export default class ParcelConfig {
         '/packagers',
       );
     }
-    return this.loadPlugin<Packager<mixed, mixed>>(packager);
+    return nullthrows(await this.loadPlugin<Packager<mixed, mixed>>(packager));
   }
 
   _getOptimizerNodes(

diff --git a/packages/core/core/src/assetUtils.js b/packages/core/core/src/assetUtils.js
@@ -152,14 +152,16 @@ async function _generateFromAST(asset: CommittedAsset | UncommittedAsset) {
   }
 
   let pluginName = nullthrows(asset.value.plugin);
-  let {plugin} = await loadPlugin<Transformer<mixed>>(
-    pluginName,
-    fromProjectPath(
-      asset.options.projectRoot,
-      nullthrows(asset.value.configPath),
+  let {plugin} = nullthrows(
+    await loadPlugin<Transformer<mixed>>(
+      pluginName,
+      fromProjectPath(
+        asset.options.projectRoot,
+        nullthrows(asset.value.configPath),
+      ),
+      nullthrows(asset.value.configKeyPath),
+      asset.options,
     ),
-    nullthrows(asset.value.configKeyPath),
-    asset.options,
   );
   let generate = plugin.generate?.bind(plugin);
   if (!generate) {

diff --git a/packages/core/core/src/loadParcelPlugin.js b/packages/core/core/src/loadParcelPlugin.js
@@ -31,10 +31,15 @@ export default async function loadPlugin<T>(
   version: Semver,
   resolveFrom: ProjectPath,
   range: ?SemverRange,
-|}> {
+|} | null> {
   let resolveFrom = configPath;
   let range;
-  if (resolveFrom.includes(NODE_MODULES)) {
+  let isOptional = false;
+  if (
+    resolveFrom.includes(NODE_MODULES) ||
+    (process.env.PARCEL_BUILD_ENV !== 'production' &&
+      /packages[/\\]configs/.test(resolveFrom))
+  ) {
     // Config packages can reference plugins, but cannot contain other plugins within them.
     // This forces every published plugin to be published separately so they can be mixed and matched if needed.
     if (pluginName.startsWith('.')) {
@@ -75,8 +80,11 @@ export default async function loadPlugin<T>(
       // If not in the config's dependencies, the plugin will be auto installed with
       // the version declared in "parcelDependencies".
       range = configPkg.config.parcelDependencies?.[pluginName];
+      isOptional =
+        Array.isArray(configPkg.config.optionalParcelDependencies) &&
+        configPkg.config.optionalParcelDependencies.includes(pluginName);
 
-      if (range == null) {
+      if (range == null && !isOptional) {
         let contents = await options.inputFS.readFile(
           configPkg.files[0].filePath,
           'utf8',
@@ -122,7 +130,7 @@ export default async function loadPlugin<T>(
       pluginName,
       resolveFrom,
       {
-        shouldAutoInstall: options.shouldAutoInstall,
+        shouldAutoInstall: options.shouldAutoInstall && !isOptional,
         range,
       },
     ));
@@ -131,6 +139,10 @@ export default async function loadPlugin<T>(
       throw err;
     }
 
+    if (isOptional) {
+      return null;
+    }
+
     let configContents = await options.inputFS.readFile(configPath, 'utf8');
     let alternatives = await findAlternativeNodeModules(
       options.inputFS,

diff --git a/packages/core/core/test/ParcelConfigRequest.test.js b/packages/core/core/test/ParcelConfigRequest.test.js
@@ -16,6 +16,8 @@ import {
 import {validatePackageName} from '../src/ParcelConfig.schema';
 import {DEFAULT_OPTIONS, relative} from './test-utils';
 import {toProjectPath} from '../src/projectPath';
+import json5 from 'json5';
+import fs from 'fs';
 
 describe('ParcelConfigRequest', () => {
   describe('validatePackageName', () => {
@@ -696,7 +698,7 @@ describe('ParcelConfigRequest', () => {
       let defaultConfigPath = require.resolve('@parcel/config-default');
       let defaultConfig = await processConfig(
         {
-          ...require('@parcel/config-default'),
+          ...json5.parse(fs.readFileSync(defaultConfigPath, 'utf8')),
           filePath: defaultConfigPath,
         },
         DEFAULT_OPTIONS,

diff --git a/packages/transformers/js/core/src/dependency_collector.rs b/packages/transformers/js/core/src/dependency_collector.rs
@@ -1913,7 +1913,7 @@ try {{
         kind: DependencyKind::Require,
         specifier: "other".into(),
         attributes: None,
-        flags: DependencyFlags::empty(),
+        flags: DependencyFlags::OPTIONAL,
         source_type: Some(SourceType::Module),
         placeholder: Some(hash),
         ..items[0].clone()