From aafc318def6536271093ba706fc950e3be433a40 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Thu, 7 Oct 2021 20:18:38 +0200 Subject: [PATCH] Don't use deprecated querystring package (#6806) --- packages/core/core/package.json | 1 - packages/core/core/src/AssetGraph.js | 6 ++--- packages/core/core/src/BundleGraph.js | 23 ++++++++++--------- packages/core/core/src/Transformation.js | 3 +-- packages/core/core/src/assetUtils.js | 9 ++------ packages/core/core/src/public/Asset.js | 11 +++++---- .../core/core/src/requests/AssetRequest.js | 3 +-- .../core/core/src/requests/PathRequest.js | 2 +- packages/core/core/src/types.js | 5 ++-- packages/core/core/test/AssetGraph.test.js | 10 ++------ packages/core/core/test/BundleGraph.test.js | 2 +- .../test/PublicMutableBundleGraph.test.js | 4 ++-- .../parcel-bundler-no-defer/index.js | 12 ++++++---- .../package-manager/src/NodePackageManager.js | 1 - packages/core/types/index.js | 6 ++--- .../image/src/ImageTransformer.js | 19 +++++++++------ .../utils/node-resolver-core/package.json | 3 +-- .../node-resolver-core/src/NodeResolver.js | 9 +++----- .../utils/node-resolver-core/src/builtins.js | 4 ++-- .../utils/node-resolver-core/test/resolver.js | 6 ++--- yarn.lock | 2 +- 21 files changed, 64 insertions(+), 77 deletions(-) diff --git a/packages/core/core/package.json b/packages/core/core/package.json index bd76f26b6d5..ab652393c41 100644 --- a/packages/core/core/package.json +++ b/packages/core/core/package.json @@ -47,7 +47,6 @@ "json5": "^1.0.1", "micromatch": "^4.0.2", "nullthrows": "^1.1.1", - "querystring": "^0.2.0", "semver": "^5.4.1" }, "devDependencies": { diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js index 6c2f1e93523..dbc9dfb5f26 100644 --- a/packages/core/core/src/AssetGraph.js +++ b/packages/core/core/src/AssetGraph.js @@ -19,7 +19,7 @@ import type { import invariant from 'assert'; import {hashString, Hash} from '@parcel/hash'; -import {hashObject, objectSortedEntries} from '@parcel/utils'; +import {hashObject} from '@parcel/utils'; import nullthrows from 'nullthrows'; import {ContentGraph} from '@parcel/graph'; import {createDependency} from './Dependency'; @@ -62,9 +62,7 @@ export function nodeFromAssetGroup(assetGroup: AssetGroup): AssetGroupNode { ':' + (assetGroup.pipeline ?? '') + ':' + - (assetGroup.query - ? JSON.stringify(objectSortedEntries(assetGroup.query)) - : ''), + (assetGroup.query ?? ''), ), type: 'asset_group', value: assetGroup, diff --git a/packages/core/core/src/BundleGraph.js b/packages/core/core/src/BundleGraph.js index 6436d994b51..be7390daa8e 100644 --- a/packages/core/core/src/BundleGraph.js +++ b/packages/core/core/src/BundleGraph.js @@ -7,7 +7,6 @@ import type { TraversalActions, } from '@parcel/types'; import type {NodeId, SerializedContentGraph} from '@parcel/graph'; -import querystring from 'querystring'; import type { Asset, @@ -1491,16 +1490,18 @@ export default class BundleGraph { let hash = new Hash(); // TODO: sort?? this.traverseAssets(bundle, asset => { - hash.writeString( - [ - this.getAssetPublicId(asset), - asset.outputHash, - asset.filePath, - querystring.stringify(asset.query), - asset.type, - asset.uniqueKey, - ].join(':'), - ); + { + hash.writeString( + [ + this.getAssetPublicId(asset), + asset.outputHash, + asset.filePath, + asset.query, + asset.type, + asset.uniqueKey, + ].join(':'), + ); + } }); let hashHex = hash.finish(); diff --git a/packages/core/core/src/Transformation.js b/packages/core/core/src/Transformation.js index 4511843dc92..bdd18698f51 100644 --- a/packages/core/core/src/Transformation.js +++ b/packages/core/core/src/Transformation.js @@ -24,7 +24,6 @@ import type {LoadedPlugin} from './ParcelConfig'; import path from 'path'; import {Readable} from 'stream'; import nullthrows from 'nullthrows'; -import {objectSortedEntries} from '@parcel/utils'; import logger, {PluginLogger} from '@parcel/logger'; import ThrowableDiagnostic, { errorToDiagnostic, @@ -561,7 +560,7 @@ export default class Transformation { a.value.pipeline, a.value.hash, a.value.uniqueKey, - a.value.query ? JSON.stringify(objectSortedEntries(a.value.query)) : '', + a.value.query ?? '', ]) .join(''); diff --git a/packages/core/core/src/assetUtils.js b/packages/core/core/src/assetUtils.js index 4ab3be355fc..0cf7cc44724 100644 --- a/packages/core/core/src/assetUtils.js +++ b/packages/core/core/src/assetUtils.js @@ -11,7 +11,6 @@ import type { Symbol, SourceLocation, Transformer, - QueryParameters, } from '@parcel/types'; import type { Asset, @@ -20,7 +19,6 @@ import type { Environment, ParcelOptions, } from './types'; -import {objectSortedEntries} from '@parcel/utils'; import {Readable} from 'stream'; import {PluginLogger} from '@parcel/logger'; @@ -47,7 +45,7 @@ type AssetOptions = {| hash?: ?string, idBase?: ?string, filePath: ProjectPath, - query?: ?QueryParameters, + query?: ?string, type: string, contentKey?: ?string, mapKey?: ?string, @@ -76,9 +74,6 @@ export function createAssetIdFromOptions(options: AssetOptions): string { options.idBase != null ? options.idBase : fromProjectPathRelative(options.filePath); - let queryString = options.query - ? JSON.stringify(objectSortedEntries(options.query)) - : ''; return hashString( idBase + @@ -88,7 +83,7 @@ export function createAssetIdFromOptions(options: AssetOptions): string { ':' + (options.pipeline ?? '') + ':' + - queryString, + (options.query ?? ''), ); } diff --git a/packages/core/core/src/public/Asset.js b/packages/core/core/src/public/Asset.js index d9bb86bec7d..88da55b1558 100644 --- a/packages/core/core/src/public/Asset.js +++ b/packages/core/core/src/public/Asset.js @@ -19,7 +19,6 @@ import type { Stats, MutableAssetSymbols as IMutableAssetSymbols, AssetSymbols as IAssetSymbols, - QueryParameters, BundleBehavior, } from '@parcel/types'; import type {Asset as AssetValue, ParcelOptions} from '../types'; @@ -83,13 +82,14 @@ export function assetFromValue( class BaseAsset { #asset: CommittedAsset | UncommittedAsset; + #query /*: ?URLSearchParams */; constructor(asset: CommittedAsset | UncommittedAsset) { this.#asset = asset; _assetToAssetValue.set(this, asset.value); } - // $FlowFixMe + // $FlowFixMe[unsupported-syntax] [inspect](): string { return `Asset(${this.filePath})`; } @@ -117,8 +117,11 @@ class BaseAsset { ); } - get query(): QueryParameters { - return this.#asset.value.query ?? {}; + get query(): URLSearchParams { + if (!this.#query) { + this.#query = new URLSearchParams(this.#asset.value.query ?? ''); + } + return this.#query; } get meta(): Meta { diff --git a/packages/core/core/src/requests/AssetRequest.js b/packages/core/core/src/requests/AssetRequest.js index 65293c785f1..07db6a1e69f 100644 --- a/packages/core/core/src/requests/AssetRequest.js +++ b/packages/core/core/src/requests/AssetRequest.js @@ -12,7 +12,6 @@ import type { import type {ConfigAndCachePath} from './ParcelConfigRequest'; import type {TransformationResult} from '../Transformation'; -import {objectSortedEntries} from '@parcel/utils'; import nullthrows from 'nullthrows'; import {hashString} from '@parcel/hash'; import createParcelConfigRequest from './ParcelConfigRequest'; @@ -59,7 +58,7 @@ function getId(input: AssetRequestInput) { ':' + (input.pipeline ?? '') + ':' + - (input.query ? JSON.stringify(objectSortedEntries(input.query)) : ''), + (input.query ?? ''), ); } diff --git a/packages/core/core/src/requests/PathRequest.js b/packages/core/core/src/requests/PathRequest.js index 73bdcd62c93..347fbe12468 100644 --- a/packages/core/core/src/requests/PathRequest.js +++ b/packages/core/core/src/requests/PathRequest.js @@ -239,7 +239,7 @@ export class ResolverRunner { this.options.projectRoot, resultFilePath, ), - query: result.query, + query: result.query?.toString(), sideEffects: result.sideEffects, code: result.code, env: dependency.env, diff --git a/packages/core/core/src/types.js b/packages/core/core/src/types.js index 565b43bc67a..5f1f5ddb585 100644 --- a/packages/core/core/src/types.js +++ b/packages/core/core/src/types.js @@ -24,7 +24,6 @@ import type { OutputFormat, TargetDescriptor, HMROptions, - QueryParameters, DetailedReportOptions, } from '@parcel/types'; import type {SharedReference} from '@parcel/workers'; @@ -158,7 +157,7 @@ export type Asset = {| committed: boolean, hash: ?string, filePath: ProjectPath, - query: ?QueryParameters, + query: ?string, type: string, dependencies: Map, bundleBehavior: ?$Values, @@ -330,7 +329,7 @@ export type AssetRequestInput = {| pipeline?: ?string, optionsRef: SharedReference, isURL?: boolean, - query?: ?QueryParameters, + query?: ?string, |}; export type AssetRequestResult = Array; diff --git a/packages/core/core/test/AssetGraph.test.js b/packages/core/core/test/AssetGraph.test.js index ce1ed9e500d..4f2a246f686 100644 --- a/packages/core/core/test/AssetGraph.test.js +++ b/packages/core/core/test/AssetGraph.test.js @@ -253,7 +253,6 @@ describe('AssetGraph', () => { let req = { filePath: toProjectPath('/index.js'), env: DEFAULT_ENV, - query: {}, }; graph.resolveDependency(dep, req, '3'); @@ -267,7 +266,6 @@ describe('AssetGraph', () => { let req2 = { filePath: toProjectPath('/index.jsx'), env: DEFAULT_ENV, - query: {}, }; graph.resolveDependency(dep, req2, '4'); @@ -319,7 +317,7 @@ describe('AssetGraph', () => { }); let sourcePath = '/index.js'; let filePath = toProjectPath(sourcePath); - let req = {filePath, env: DEFAULT_ENV, query: {}}; + let req = {filePath, env: DEFAULT_ENV}; graph.resolveDependency(dep, req, '3'); let assets = [ createAsset({ @@ -486,7 +484,7 @@ describe('AssetGraph', () => { }); let sourcePath = '/index.js'; let filePath = toProjectPath(sourcePath); - let req = {filePath, env: DEFAULT_ENV, query: {}}; + let req = {filePath, env: DEFAULT_ENV}; graph.resolveDependency(dep, req, '123'); let dep1 = createDependency({ specifier: 'dependent-asset-1', @@ -566,7 +564,6 @@ describe('AssetGraph', () => { let indexAssetGroup = { filePath: toProjectPath('/index.js'), env: DEFAULT_ENV, - query: {}, }; graph.setRootConnections({assetGroups: [indexAssetGroup]}); let indexFooDep = createDependency({ @@ -600,7 +597,6 @@ describe('AssetGraph', () => { let fooAssetGroup = { filePath: toProjectPath('/foo.js'), env: DEFAULT_ENV, - query: {}, }; graph.resolveDependency(indexFooDep, fooAssetGroup, '0'); let fooAssetGroupNode = nodeFromAssetGroup(fooAssetGroup); @@ -626,7 +622,6 @@ describe('AssetGraph', () => { let utilsAssetGroup = { filePath: toProjectPath('/utils.js'), env: DEFAULT_ENV, - query: {}, }; let utilsAssetGroupNode = nodeFromAssetGroup(utilsAssetGroup); graph.resolveDependency(fooUtilsDep, utilsAssetGroup, '0'); @@ -646,7 +641,6 @@ describe('AssetGraph', () => { let barAssetGroup = { filePath: toProjectPath('/bar.js'), env: DEFAULT_ENV, - query: {}, }; graph.resolveDependency(indexBarDep, barAssetGroup, '0'); let barAssetGroupNode = nodeFromAssetGroup(barAssetGroup); diff --git a/packages/core/core/test/BundleGraph.test.js b/packages/core/core/test/BundleGraph.test.js index 6df5aeef958..9d65a8e4948 100644 --- a/packages/core/core/test/BundleGraph.test.js +++ b/packages/core/core/test/BundleGraph.test.js @@ -83,7 +83,7 @@ function createMockAssetGraph(ids: [string, string]) { }); let sourcePath = '/index.js'; let filePath = toProjectPath('/', sourcePath); - let req = {filePath, env: DEFAULT_ENV, query: {}}; + let req = {filePath, env: DEFAULT_ENV}; graph.resolveDependency(dep, nodeFromAssetGroup(req).value, '3'); let dep1 = createDependency({ diff --git a/packages/core/core/test/PublicMutableBundleGraph.test.js b/packages/core/core/test/PublicMutableBundleGraph.test.js index 58328f17aa5..20fe2fae955 100644 --- a/packages/core/core/test/PublicMutableBundleGraph.test.js +++ b/packages/core/core/test/PublicMutableBundleGraph.test.js @@ -151,7 +151,7 @@ function createMockAssetGraph() { }); let filePath = toProjectPath('/', '/index.js'); - let req1 = {filePath, env: DEFAULT_ENV, query: {}}; + let req1 = {filePath, env: DEFAULT_ENV}; graph.resolveDependency(dep1, nodeFromAssetGroup(req1).value, '5'); graph.resolveAssetGroup( req1, @@ -170,7 +170,7 @@ function createMockAssetGraph() { ); filePath = toProjectPath('/', '/index2.js'); - let req2 = {filePath, env: DEFAULT_ENV, query: {}}; + let req2 = {filePath, env: DEFAULT_ENV}; graph.resolveDependency(dep2, nodeFromAssetGroup(req2).value, '7'); graph.resolveAssetGroup( req2, diff --git a/packages/core/integration-tests/test/integration/resolver-canDefer/node_modules/parcel-bundler-no-defer/index.js b/packages/core/integration-tests/test/integration/resolver-canDefer/node_modules/parcel-bundler-no-defer/index.js index a1d638fbf21..5e17cf21bce 100644 --- a/packages/core/integration-tests/test/integration/resolver-canDefer/node_modules/parcel-bundler-no-defer/index.js +++ b/packages/core/integration-tests/test/integration/resolver-canDefer/node_modules/parcel-bundler-no-defer/index.js @@ -90,11 +90,12 @@ module.exports = (new Bundler({ for (let asset of assets) { let bundle = bundleGraph.createBundle({ entryAsset: asset, - isEntry: + needsStableName: + dependency.bundleBehavior === 'inline' || asset.bundleBehavior === 'inline' ? false : dependency.isEntry || dependency.needsStableName, - isInline: asset.bundleBehavior === 'inline', + bundleBehavior: dependency.bundleBehavior || asset.bundleBehavior, target: bundleGroup.target, }); bundleByType.set(bundle.type, bundle); @@ -147,13 +148,14 @@ module.exports = (new Bundler({ env: asset.env, type: asset.type, target: bundleGroup.target, - isEntry: + needsStableName: asset.bundleBehavior === 'inline' || + dependency.bundleBehavior === 'inline' || (dependency.priority === 'parallel' && !dependency.needsStableName) ? false - : parentBundle.isEntry, - isInline: asset.bundleBehavior === 'inline', + : parentBundle.needsStableName, + bundleBehavior: dependency.bundleBehavior || asset.bundleBehavior, isSplittable: asset.isBundleSplittable || true, pipeline: asset.pipeline, }); diff --git a/packages/core/package-manager/src/NodePackageManager.js b/packages/core/package-manager/src/NodePackageManager.js index 97756a20752..cb6273187a4 100644 --- a/packages/core/package-manager/src/NodePackageManager.js +++ b/packages/core/package-manager/src/NodePackageManager.js @@ -18,7 +18,6 @@ import ThrowableDiagnostic, { md, } from '@parcel/diagnostic'; import nativeFS from 'fs'; -// $FlowFixMe this is untyped import Module from 'module'; import path from 'path'; import semver from 'semver'; diff --git a/packages/core/types/index.js b/packages/core/types/index.js index 25cf7a0b0b8..9c8bb23250c 100644 --- a/packages/core/types/index.js +++ b/packages/core/types/index.js @@ -25,8 +25,6 @@ export type ConfigResultWithFilePath = {| /** process.env */ export type EnvMap = typeof process.env; -export type QueryParameters = {[key: string]: string, ...}; - export type JSONValue = | null | void // ? Is this okay? @@ -634,7 +632,7 @@ export interface BaseAsset { */ +type: string; /** The transformer options for the asset from the dependency query string. */ - +query: QueryParameters; + +query: URLSearchParams; /** The environment of the asset. */ +env: Environment; /** @@ -1455,7 +1453,7 @@ export type ResolveResult = {| /** An optional named pipeline to use to compile the resolved file. */ +pipeline?: ?string, /** Query parameters to be used by transformers when compiling the resolved file. */ - +query?: QueryParameters, + +query?: URLSearchParams, /** Whether the resolved file should be excluded from the build. */ +isExcluded?: boolean, /** Overrides the priority set on the dependency. */ diff --git a/packages/transformers/image/src/ImageTransformer.js b/packages/transformers/image/src/ImageTransformer.js index 4a76b49fb20..2c24d470452 100644 --- a/packages/transformers/image/src/ImageTransformer.js +++ b/packages/transformers/image/src/ImageTransformer.js @@ -45,14 +45,19 @@ export default (new Transformer({ ); } - const width = asset.query.width ? parseInt(asset.query.width, 10) : null; - const height = asset.query.height ? parseInt(asset.query.height, 10) : null; - const quality = asset.query.quality - ? parseInt(asset.query.quality, 10) - : config.quality; - let targetFormat = asset.query.as - ? asset.query.as.toLowerCase().trim() + const width = asset.query.has('width') + ? parseInt(asset.query.get('width'), 10) + : null; + const height = asset.query.has('height') + ? parseInt(asset.query.get('height'), 10) : null; + const quality = asset.query.has('quality') + ? parseInt(asset.query.get('quality'), 10) + : config.quality; + let targetFormat = asset.query + .get('as') + ?.toLowerCase() + .trim(); if (targetFormat && !FORMATS.has(targetFormat)) { throw new Error( `The image transformer does not support ${targetFormat} images.`, diff --git a/packages/utils/node-resolver-core/package.json b/packages/utils/node-resolver-core/package.json index ff8b9502b3c..8a0b3140d43 100644 --- a/packages/utils/node-resolver-core/package.json +++ b/packages/utils/node-resolver-core/package.json @@ -23,7 +23,6 @@ "@parcel/node-libs-browser": "2.0.0-rc.0", "@parcel/utils": "2.0.0-rc.0", "micromatch": "^3.0.4", - "nullthrows": "^1.1.1", - "querystring": "^0.2.0" + "nullthrows": "^1.1.1" } } diff --git a/packages/utils/node-resolver-core/src/NodeResolver.js b/packages/utils/node-resolver-core/src/NodeResolver.js index 107011841c9..469076fef86 100644 --- a/packages/utils/node-resolver-core/src/NodeResolver.js +++ b/packages/utils/node-resolver-core/src/NodeResolver.js @@ -6,7 +6,6 @@ import type { ResolveResult, Environment, SpecifierType, - QueryParameters, } from '@parcel/types'; import type {FileSystem} from '@parcel/fs'; @@ -27,10 +26,8 @@ import ThrowableDiagnostic, { import micromatch from 'micromatch'; import builtins, {empty} from './builtins'; import nullthrows from 'nullthrows'; -// $FlowFixMe this is untyped import _Module from 'module'; import {fileURLToPath} from 'url'; -import {parse as parseQueryString} from 'querystring'; const EMPTY_SHIM = require.resolve('./_empty'); @@ -61,7 +58,7 @@ type Module = {| moduleDir?: FilePath, filePath?: FilePath, code?: string, - query?: QueryParameters, + query?: URLSearchParams, |}; type ResolverContext = {| @@ -420,7 +417,7 @@ export default class NodeResolver { filename: string, dir: string, specifierType: SpecifierType, - ): Promise { + ): Promise { let url; switch (filename[0]) { case '/': { @@ -526,7 +523,7 @@ export default class NodeResolver { return { filePath, - query: url.search ? parseQueryString(url.search.slice(1)) : undefined, + query: url.search ? new URLSearchParams(url.search) : undefined, }; } else { // CommonJS specifier. Query params are not supported. diff --git a/packages/utils/node-resolver-core/src/builtins.js b/packages/utils/node-resolver-core/src/builtins.js index 394997ae7ca..9bc5b16470d 100644 --- a/packages/utils/node-resolver-core/src/builtins.js +++ b/packages/utils/node-resolver-core/src/builtins.js @@ -1,6 +1,6 @@ -// @flow -import polyfills from '@parcel/node-libs-browser'; +// @flow strict-local // $FlowFixMe this is untyped +import polyfills from '@parcel/node-libs-browser'; import {builtinModules} from 'module'; export const empty: string = require.resolve('./_empty.js'); diff --git a/packages/utils/node-resolver-core/test/resolver.js b/packages/utils/node-resolver-core/test/resolver.js index 9746ae63d55..bc6952f3c16 100644 --- a/packages/utils/node-resolver-core/test/resolver.js +++ b/packages/utils/node-resolver-core/test/resolver.js @@ -259,7 +259,7 @@ describe('resolver', function() { nullthrows(resolved).filePath, path.join(rootDir, 'nested', 'index.js'), ); - assert.deepEqual(nullthrows(resolved).query, {foo: 'bar'}); + assert.deepEqual(nullthrows(resolved).query?.toString(), 'foo=bar'); }); it('should not support query params for CommonJS specifiers', async function() { @@ -1051,7 +1051,7 @@ describe('resolver', function() { nullthrows(resolved).filePath, path.resolve(rootDir, 'node_modules/@scope/pkg/index.js'), ); - assert.deepEqual(nullthrows(resolved).query, {foo: '2'}); + assert.deepEqual(nullthrows(resolved).query?.toString(), 'foo=2'); }); it('should not support query params for bare CommonJS specifiers', async function() { @@ -1080,7 +1080,7 @@ describe('resolver', function() { nullthrows(resolved).filePath, path.resolve(rootDir, 'node_modules/@scope/pkg/index.js'), ); - assert.deepEqual(nullthrows(resolved).query, {foo: '2'}); + assert.deepEqual(nullthrows(resolved).query?.toString(), 'foo=2'); }); }); diff --git a/yarn.lock b/yarn.lock index cf5a9575b12..920c90d851c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10971,7 +10971,7 @@ querystring-es3@^0.2.1: resolved "https://registry.yarnpkg.com/querystring-es3/-/querystring-es3-0.2.1.tgz#9ec61f79049875707d69414596fd907a4d711e73" integrity sha1-nsYfeQSYdXB9aUFFlv2Qek1xHnM= -querystring@0.2.0, querystring@^0.2.0: +querystring@0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/querystring/-/querystring-0.2.0.tgz#b209849203bb25df820da756e747005878521620" integrity sha1-sgmEkgO7Jd+CDadW50cAWHhSFiA=