Repository: parcel-bundler/parcel (js)

Issue:
Importing an asset twice (normal and `bundle-text`) doesn't work
# 🐛 bug report

With scopehoisting, `style` in the JS file becomes `{}`.
Without scope-hoisting, `import "./style.css";` is essentially ignored.

## 💻 Code Sample

```html
<script src="index.js"></script>
```

```js
import "./style.css";
import style from "bundle-text:./style.css";

console.log(style);
```

## 🌍 Your Environment

<!--- Include as many relevant details about the environment you experienced the bug in -->

| Software         | Version(s) |
| ---------------- | ---------- |
| Parcel           | v2


Pull request:
Support importing the same file with multiple pipelines
Fixes #4709.

This makes it possible to import the same file with multiple named pipelines, or both with and without a named pipeline. In order to do this, I needed to do three things:

1. Add the pipeline to the asset id hash.
2. Only set the named pipeline based on the target name if it actually exists in the parcel config.
3. Handle dependency `isURL` property in core rather than implicitly requiring a named `url` pipeline. This adds an `isURL` property to the AssetRequest type, and handles that in the transformation logic to allow no transformers to be defined in the config when that flag is set and just pass through the asset un-transformed.

Patch:
diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js
index a2b0061a7..cc5741f78 100644
--- a/packages/core/core/src/AssetGraph.js
+++ b/packages/core/core/src/AssetGraph.js
@@ -46,7 +46,11 @@ export function nodeFromDep(dep: Dependency): DependencyNode {
 
 export function nodeFromAssetGroup(assetGroup: AssetGroup) {
   return {
-    id: md5FromObject(assetGroup),
+    id: md5FromObject({
+      ...assetGroup,
+      // if only the isURL property is different, no need to re-run transformation.
+      isURL: undefined,
+    }),
     type: 'asset_group',
     value: assetGroup,
   };
@@ -154,7 +158,7 @@ export default class AssetGraph extends Graph<AssetGraphNode> {
       nodeFromDep(
         createDependency({
           moduleSpecifier: entry.filePath,
-          pipeline: target.name,
+          pipeline: target.pipeline,
           target: target,
           env: target.env,
           isEntry: true,
diff --git a/packages/core/core/src/ConfigLoader.js b/packages/core/core/src/ConfigLoader.js
index dfdc9370e..eff6917ba 100644
--- a/packages/core/core/src/ConfigLoader.js
+++ b/packages/core/core/src/ConfigLoader.js
@@ -47,7 +47,7 @@ export default class ConfigLoader {
   }
 
   loadParcelConfig(configRequest: ConfigRequestDesc) {
-    let {filePath, isSource, env, pipeline} = configRequest;
+    let {filePath, isSource, env, pipeline, isURL} = configRequest;
     let dir = isSource ? path.dirname(filePath) : this.options.projectRoot;
     let searchPath = path.join(dir, 'index');
     let config = createConfig({
@@ -60,7 +60,11 @@ export default class ConfigLoader {
     let devDeps = [];
     switch (configRequest.meta.actionType) {
       case 'transformation':
-        devDeps = this.parcelConfig.getTransformerNames(filePath, pipeline);
+        devDeps = this.parcelConfig.getTransformerNames(
+          filePath,
+          pipeline,
+          isURL,
+        );
         break;
       case 'validation':
         devDeps = this.parcelConfig.getValidatorNames(filePath);
diff --git a/packages/core/core/src/ParcelConfig.js b/packages/core/core/src/ParcelConfig.js
index 92e6c7b59..5fa9b1e6a 100644
--- a/packages/core/core/src/ParcelConfig.js
+++ b/packages/core/core/src/ParcelConfig.js
@@ -186,6 +186,7 @@ export default class ParcelConfig {
   _getTransformerNodes(
     filePath: FilePath,
     pipeline?: ?string,
+    allowEmpty?: boolean,
   ): $ReadOnlyArray<ParcelPluginNode> {
     let transformers: PureParcelConfigPipeline | null = this.matchGlobMapPipelines(
       filePath,
@@ -193,20 +194,36 @@ export default class ParcelConfig {
       pipeline,
     );
     if (!transformers || transformers.length === 0) {
+      if (allowEmpty) {
+        return [];
+      }
+
       throw new Error(`No transformers found for "${filePath}".`);
     }
 
     return transformers;
   }
 
-  getTransformerNames(filePath: FilePath, pipeline?: ?string): Array<string> {
-    let transformers = this._getTransformerNodes(filePath, pipeline);
+  getTransformerNames(
+    filePath: FilePath,
+    pipeline?: ?string,
+    allowEmpty?: boolean,
+  ): Array<string> {
+    let transformers = this._getTransformerNodes(
+      filePath,
+      pipeline,
+      allowEmpty,
+    );
     return transformers.map(t => t.packageName);
   }
 
-  getTransformers(filePath: FilePath, pipeline?: ?string) {
+  getTransformers(
+    filePath: FilePath,
+    pipeline?: ?string,
+    allowEmpty?: boolean,
+  ) {
     return this.loadPlugins<Transformer>(
-      this._getTransformerNodes(filePath, pipeline),
+      this._getTransformerNodes(filePath, pipeline, allowEmpty),
     );
   }
 
diff --git a/packages/core/core/src/Transformation.js b/packages/core/core/src/Transformation.js
index e03fe04c2..0a528c963 100644
--- a/packages/core/core/src/Transformation.js
+++ b/packages/core/core/src/Transformation.js
@@ -114,7 +114,7 @@ export default class Transformation {
     let pipeline = await this.loadPipeline(
       this.request.filePath,
       asset.value.isSource,
-      this.request.pipeline,
+      this.request,
     );
     let results = await this.runPipelines(pipeline, asset);
     let assets = results.map(a => a.value);
@@ -253,6 +253,10 @@ export default class Transformation {
     pipeline: Pipeline,
     initialAsset: UncommittedAsset,
   ): Promise<Array<UncommittedAsset>> {
+    if (pipeline.transformers.length === 0) {
+      return [initialAsset];
+    }
+
     let initialType = initialAsset.value.type;
     let inputAssets = [initialAsset];
     let resultingAssets = [];
@@ -391,13 +395,14 @@ export default class Transformation {
   async loadPipeline(
     filePath: FilePath,
     isSource: boolean,
-    pipelineName?: ?string,
+    request?: AssetRequestInput,
   ): Promise<Pipeline> {
     let configRequest = {
       filePath,
       env: this.request.env,
       isSource,
-      pipeline: pipelineName,
+      pipeline: request?.pipeline,
+      isURL: request?.isURL,
       meta: {
         actionType: 'transformation',
       },
@@ -410,7 +415,8 @@ export default class Transformation {
 
     let transformers = await this.parcelConfig.getTransformers(
       filePath,
-      pipelineName,
+      request?.pipeline,
+      request?.isURL,
     );
 
     for (let {name, resolveFrom} of transformers) {
@@ -461,7 +467,7 @@ export default class Transformation {
     let nextPipeline = await this.loadPipeline(
       nextFilePath,
       isSource,
-      this.request.pipeline,
+      this.request,
     );
 
     if (nextPipeline.id === currentPipeline.id) {
diff --git a/packages/core/core/src/assetUtils.js b/packages/core/core/src/assetUtils.js
index 915ea9e71..41bfea936 100644
--- a/packages/core/core/src/assetUtils.js
+++ b/packages/core/core/src/assetUtils.js
@@ -63,7 +63,11 @@ export function createAsset(options: AssetOptions): Asset {
       options.id != null
         ? options.id
         : md5FromString(
-            idBase + options.type + getEnvironmentHash(options.env) + uniqueKey,
+            idBase +
+              options.type +
+              getEnvironmentHash(options.env) +
+              uniqueKey +
+              (options.pipeline ?? ''),
           ),
     committed: options.committed ?? false,
     hash: options.hash,
diff --git a/packages/core/core/src/requests/PathRequest.js b/packages/core/core/src/requests/PathRequest.js
index e3c7a3941..27ef8da5c 100644
--- a/packages/core/core/src/requests/PathRequest.js
+++ b/packages/core/core/src/requests/PathRequest.js
@@ -146,9 +146,6 @@ export class ResolverRunner {
         throw new Error(`Received URL without a pathname ${filePath}.`);
       }
       filePath = decodeURIComponent(parsed.pathname);
-      if (pipeline == null) {
-        pipeline = 'url';
-      }
     }
 
     let errors: Array<ThrowableDiagnostic> = [];
@@ -172,6 +169,7 @@ export class ResolverRunner {
             code: result.code,
             env: dependency.env,
             pipeline: pipeline ?? dependency.pipeline,
+            isURL: dependency.isURL,
           };
         }
       } catch (e) {
diff --git a/packages/core/core/src/requests/TargetRequest.js b/packages/core/core/src/requests/TargetRequest.js
index 32c7be993..b0b0fb50f 100644
--- a/packages/core/core/src/requests/TargetRequest.js
+++ b/packages/core/core/src/requests/TargetRequest.js
@@ -11,6 +11,7 @@ import type {
 } from '@parcel/types';
 import type {StaticRunOpts} from '../RequestTracker';
 import type {Entry, ParcelOptions, Target} from '../types';
+import type {ConfigAndCachePath} from './ParcelConfigRequest';
 
 import ThrowableDiagnostic, {
   generateJSONCodeHighlights,
@@ -19,11 +20,14 @@ import ThrowableDiagnostic, {
 import path from 'path';
 import {loadConfig, md5FromObject, validateSchema} from '@parcel/utils';
 import {createEnvironment} from '../Environment';
+import createParcelConfigRequest from './ParcelConfigRequest';
+import ParcelConfig from '../ParcelConfig';
 // $FlowFixMe
 import browserslist from 'browserslist';
 // $FlowFixMe
 import jsonMap from 'json-source-map';
 import invariant from 'assert';
+import nullthrows from 'nullthrows';
 import {
   COMMON_TARGET_DESCRIPTOR_SCHEMA,
   DESCRIPTOR_SCHEMA,
@@ -81,6 +85,24 @@ async function run({input, api, options}: RunOpts) {
   let targetResolver = new TargetResolver(options);
   let result = await targetResolver.resolve(input.packagePath);
 
+  let {config} = nullthrows(
+    await api.runRequest<null, ConfigAndCachePath>(createParcelConfigRequest()),
+  );
+
+  let parcelConfig = new ParcelConfig(
+    config,
+    options.packageManager,
+    options.autoinstall,
+  );
+
+  // Find named pipelines for each target.
+  let pipelineNames = new Set(parcelConfig.getNamedPipelines());
+  for (let target of result.targets) {
+    if (pipelineNames.has(target.name)) {
+      target.pipeline = target.name;
+    }
+  }
+
   // Connect files like package.json that affect the target
   // resolution so we invalidate when they change.
   for (let file of result.files) {
diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js
index 9bf346cea..5280d1efc 100644
--- a/packages/core/core/src/types.js
+++ b/packages/core/core/src/types.js
@@ -80,6 +80,7 @@ export type Target = {|
   name: string,
   publicUrl: string,
   loc?: ?SourceLocation,
+  pipeline?: string,
 |};
 
 export type Dependency = {|
@@ -208,6 +209,7 @@ export type AssetRequestInput = {|
   code?: string,
   pipeline?: ?string,
   optionsRef: number,
+  isURL?: boolean,
 |};
 
 export type AssetRequestResult = Array<Asset>;
@@ -297,6 +299,7 @@ export type ConfigRequestDesc = {|
   env: Environment,
   isSource: boolean,
   pipeline?: ?string,
+  isURL?: boolean,
   plugin?: PackageName,
   meta: JSONObject,
 |};


Test patch:
diff --git a/packages/core/core/test/AssetGraph.test.js b/packages/core/core/test/AssetGraph.test.js
index a5ef06010..bf5f2ac99 100644
--- a/packages/core/core/test/AssetGraph.test.js
+++ b/packages/core/core/test/AssetGraph.test.js
@@ -105,7 +105,6 @@ describe('AssetGraph', () => {
       graph.nodes.has(
         createDependency({
           moduleSpecifier: '/path/to/index1/src/main.js',
-          pipeline: 'test',
           target: DEFAULT_TARGETS[0],
           env: DEFAULT_ENV,
         }).id,
@@ -115,7 +114,6 @@ describe('AssetGraph', () => {
       graph.nodes.has(
         createDependency({
           moduleSpecifier: '/path/to/index2/src/main.js',
-          pipeline: 'test',
           target: DEFAULT_TARGETS[0],
           env: DEFAULT_ENV,
         }).id,
@@ -155,7 +153,6 @@ describe('AssetGraph', () => {
         }).id,
         to: createDependency({
           moduleSpecifier: '/path/to/index1/src/main.js',
-          pipeline: 'test',
           target: DEFAULT_TARGETS[0],
           env: DEFAULT_ENV,
         }).id,
@@ -168,7 +165,6 @@ describe('AssetGraph', () => {
         }).id,
         to: createDependency({
           moduleSpecifier: '/path/to/index2/src/main.js',
-          pipeline: 'test',
           target: DEFAULT_TARGETS[0],
           env: DEFAULT_ENV,
         }).id,
@@ -197,7 +193,6 @@ describe('AssetGraph', () => {
 
     let dep = createDependency({
       moduleSpecifier: '/path/to/index/src/main.js',
-      pipeline: 'test',
       target: DEFAULT_TARGETS[0],
       env: DEFAULT_ENV,
     });
@@ -239,7 +234,6 @@ describe('AssetGraph', () => {
 
     let dep = createDependency({
       moduleSpecifier: '/path/to/index/src/main.js',
-      pipeline: 'test',
       target: DEFAULT_TARGETS[0],
       env: DEFAULT_ENV,
       sourcePath: '',
@@ -381,7 +375,6 @@ describe('AssetGraph', () => {
 
     let dep = createDependency({
       moduleSpecifier: '/path/to/index/src/main.js',
-      pipeline: 'test',
       env: DEFAULT_ENV,
       target: DEFAULT_TARGETS[0],
     });
diff --git a/packages/core/core/test/BundleGraph.test.js b/packages/core/core/test/BundleGraph.test.js
index 5ffcd1dcd..ca5f7980d 100644
--- a/packages/core/core/test/BundleGraph.test.js
+++ b/packages/core/core/test/BundleGraph.test.js
@@ -60,7 +60,6 @@ function createMockAssetGraph(ids: [string, string]) {
 
   let dep = createDependency({
     moduleSpecifier: '/path/to/index/src/main.js',
-    pipeline: 'test',
     env: DEFAULT_ENV,
     target: DEFAULT_TARGETS[0],
   });
diff --git a/packages/core/core/test/PublicMutableBundleGraph.test.js b/packages/core/core/test/PublicMutableBundleGraph.test.js
index 019d80e43..f5d61e2db 100644
--- a/packages/core/core/test/PublicMutableBundleGraph.test.js
+++ b/packages/core/core/test/PublicMutableBundleGraph.test.js
@@ -81,14 +81,12 @@ function createMockAssetGraph() {
   let dep1 = createDependency({
     moduleSpecifier: '/path/to/index/src/main.js',
     isEntry: true,
-    pipeline: 'test',
     env: DEFAULT_ENV,
     target: DEFAULT_TARGETS[0],
   });
   let dep2 = createDependency({
     moduleSpecifier: '/path/to/index/src/main2.js',
     isEntry: true,
-    pipeline: 'test',
     env: DEFAULT_ENV,
     target: DEFAULT_TARGETS[0],
   });
diff --git a/packages/core/integration-tests/test/integration/multi-pipeline/index.js b/packages/core/integration-tests/test/integration/multi-pipeline/index.js
new file mode 100644
index 000000000..618abe436
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/multi-pipeline/index.js
@@ -0,0 +1,5 @@
+import "./style.css";
+import style from "bundle-text:./style.css";
+
+export default style;
+
diff --git a/packages/core/integration-tests/test/integration/multi-pipeline/style.css b/packages/core/integration-tests/test/integration/multi-pipeline/style.css
new file mode 100644
index 000000000..5a853caf0
--- /dev/null
+++ b/packages/core/integration-tests/test/integration/multi-pipeline/style.css
@@ -0,0 +1,3 @@
+.test {
+  background: red;
+}
diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js
index 12d62a54f..183c06eec 100644
--- a/packages/core/integration-tests/test/javascript.js
+++ b/packages/core/integration-tests/test/javascript.js
@@ -2219,10 +2219,34 @@ describe('javascript', function() {
     let b = await bundle(
       path.join(__dirname, '/integration/data-url/binary.js'),
     );
+    ``;
 
     assert((await run(b)).default.startsWith('data:image/webp;base64,UklGR'));
   });
 
+  it('should support both pipeline and non-pipeline imports', async () => {
+    let b = await bundle(
+      path.join(__dirname, '/integration/multi-pipeline/index.js'),
+    );
+
+    assertBundles(b, [
+      {
+        name: 'index.js',
+        assets: ['index.js', b.getBundles().find(b => b.isInline).id + '.js'],
+      },
+      {
+        name: 'index.css',
+        assets: ['style.css'],
+      },
+      {
+        type: 'css',
+        assets: ['style.css'],
+      },
+    ]);
+
+    assert((await run(b)).default.startsWith('.test'));
+  });
+
   it('should detect typescript style async requires in commonjs', async () => {
     let b = await bundle(
       path.join(__dirname, '/integration/require-async/ts.js'),