Skip to content

Commit

Permalink
Cache and asset graph bugfixes for query parameters (#5487)
Browse files Browse the repository at this point in the history
* add integration test

* dont randomly remove query params...

* add query to cache key

* add query to asset id

* bugfix

* remove .only

* tweak

* fix flow idk flow is weird af

* cache test

* sort query params in cachekey
  • Loading branch information
DeMoorJasper committed Dec 31, 2020
1 parent 0cd78e2 commit 303a312
Show file tree
Hide file tree
Showing 9 changed files with 91 additions and 16 deletions.
3 changes: 3 additions & 0 deletions packages/core/core/src/Transformation.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
escapeMarkdown,
md5FromOrderedObject,
normalizeSeparators,
objectSortedEntries,
} from '@parcel/utils';
import logger, {PluginLogger} from '@parcel/logger';
import {init as initSourcemaps} from '@parcel/source-map';
Expand Down Expand Up @@ -470,6 +471,7 @@ export default class Transformation {
pipeline: a.value.pipeline,
hash: a.value.hash,
uniqueKey: a.value.uniqueKey,
query: a.value.query ? objectSortedEntries(a.value.query) : '',
}));

return md5FromOrderedObject({
Expand Down Expand Up @@ -749,6 +751,7 @@ function normalizeAssets(
return {
ast: internalAsset.ast,
content: await internalAsset.content,
query: internalAsset.value.query,
// $FlowFixMe
dependencies: [...internalAsset.value.dependencies.values()],
env: internalAsset.value.env,
Expand Down
1 change: 1 addition & 0 deletions packages/core/core/src/UncommittedAsset.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ export default class UncommittedAsset {
hash: this.value.hash,
filePath: this.value.filePath,
type: result.type,
query: result.query,
isIsolated: result.isIsolated ?? this.value.isIsolated,
isInline: result.isInline ?? this.value.isInline,
isSplittable: result.isSplittable ?? this.value.isSplittable,
Expand Down
33 changes: 19 additions & 14 deletions packages/core/core/src/assetUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
Environment,
ParcelOptions,
} from './types';
import {objectSortedEntries} from '@parcel/utils';
import type {ConfigOutput} from '@parcel/utils';

import {Readable} from 'stream';
Expand All @@ -33,7 +34,7 @@ import PluginOptions from './public/PluginOptions';
import {
blobToStream,
loadConfig,
md5FromString,
md5FromOrderedObject,
md5FromFilePath,
} from '@parcel/utils';
import {hashFromOption} from './utils';
Expand Down Expand Up @@ -68,20 +69,24 @@ type AssetOptions = {|
configKeyPath?: string,
|};

export function createAsset(options: AssetOptions): Asset {
function createAssetIdFromOptions(options: AssetOptions): string {
let uniqueKey = options.uniqueKey ?? '';
let idBase = options.idBase != null ? options.idBase : options.filePath;
let uniqueKey = options.uniqueKey || '';
let queryString = options.query ? objectSortedEntries(options.query) : '';

return md5FromOrderedObject({
idBase,
type: options.type,
env: options.env.id,
uniqueKey,
pipeline: options.pipeline ?? '',
queryString,
});
}

export function createAsset(options: AssetOptions): Asset {
return {
id:
options.id != null
? options.id
: md5FromString(
idBase +
options.type +
options.env.id +
uniqueKey +
(options.pipeline ?? ''),
),
id: options.id != null ? options.id : createAssetIdFromOptions(options),
committed: options.committed ?? false,
hash: options.hash,
filePath: options.filePath,
Expand All @@ -103,7 +108,7 @@ export function createAsset(options: AssetOptions): Asset {
stats: options.stats,
symbols: options.symbols,
sideEffects: options.sideEffects ?? true,
uniqueKey: uniqueKey,
uniqueKey: options.uniqueKey ?? '',
plugin: options.plugin,
configPath: options.configPath,
configKeyPath: options.configKeyPath,
Expand Down
37 changes: 36 additions & 1 deletion packages/core/integration-tests/test/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -2448,7 +2448,7 @@ describe('cache', function() {
await overlayFS.writeFile(
path.join(inputDir, 'src/index.js'),
`import React from 'react';
export function Component() {
return <h1>Hello world</h1>;
}`,
Expand Down Expand Up @@ -2601,4 +2601,39 @@ describe('cache', function() {
assert(contents.includes('UPDATED CODE'));
});
});

describe('Query Parameters', () => {
it('Should create additional assets if multiple query parameter combinations are used', async function() {
let b = await testCache(
{
entries: ['reformat.html'],
update: async b => {
let bundles = b.bundleGraph.getBundles();
let contents = await overlayFS.readFile(
bundles[0].filePath,
'utf8',
);
assert(contents.includes('.webp" alt="test image">'));
assert.equal(bundles.length, 2);
await overlayFS.writeFile(
path.join(inputDir, 'reformat.html'),
`<picture>
<source src="url:./image.jpg?as=webp&width=400" type="image/webp" />
<source src="url:./image.jpg?as=jpg&width=400" type="image/jpeg" />
<img src="url:./image.jpg?as=jpg&width=800" alt="test image" />
</picture>`,
);
},
},
'image',
);

let bundles = b.bundleGraph.getBundles();
let contents = await overlayFS.readFile(bundles[0].filePath, 'utf8');
assert(contents.includes('.webp" type="image/webp">'));
assert(contents.includes('.jpg" type="image/jpeg">'));
assert(contents.includes('.jpg" alt="test image">'));
assert.equal(bundles.length, 4);
});
});
});
20 changes: 20 additions & 0 deletions packages/core/integration-tests/test/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,26 @@ describe('image', function() {
assert.equal(image.width, 600);
});

it('Should be able to import an image using multiple varying query parameters', async () => {
await bundle(
path.join(__dirname, '/integration/image-multiple-queries/index.html'),
);

let dirContent = await outputFS.readdir(distDir);
let foundExtensions = [];
for (let filename of dirContent) {
const foundExt = path.extname(filename);
if (foundExt !== '.map') {
foundExtensions.push(foundExt);
}
}

assert.deepStrictEqual(
foundExtensions.sort(),
['.jpg', '.jpg', '.webp', '.html'].sort(),
);
});

describe('Should be able to change image format', () => {
function testCase(ext) {
return async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!DOCTYPE html>
<html lang="en">
<body>
<picture>
<source src="url:./snow.jpg?as=webp&width=400" type="image/webp" />
<source src="url:./snow.jpg?as=jpg&width=400" type="image/jpeg" />
<img src="url:./snow.jpg?as=jpg&width=800" alt="snow" />
</picture>
</body>
</html>
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<img src="url:./image.jpg?as=webp"/>
<img src="url:./image.jpg?as=webp" alt="test image" />
1 change: 1 addition & 0 deletions packages/core/types/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,7 @@ export type TransformerResult = {|
+dependencies?: $ReadOnlyArray<DependencyOptions>,
+env?: EnvironmentOpts,
+filePath?: FilePath,
+query?: ?QueryParameters,
+includedFiles?: $ReadOnlyArray<File>,
+isInline?: boolean,
+isIsolated?: boolean,
Expand Down

0 comments on commit 303a312

Please sign in to comment.