From e7d212b17942d47f6f0ed7e103ecf47a5cbb2c67 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 17 Sep 2024 10:11:24 -0600 Subject: [PATCH 1/8] chore: cache walkContent --- src/resolve/sourceComponent.ts | 15 +++++++++------ test/resolve/adapters/bundleSourceAdapter.test.ts | 6 ++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/resolve/sourceComponent.ts b/src/resolve/sourceComponent.ts index 16d9afa955..c67ca7e293 100644 --- a/src/resolve/sourceComponent.ts +++ b/src/resolve/sourceComponent.ts @@ -54,6 +54,7 @@ export class SourceComponent implements MetadataComponent { private markedForDelete = false; private destructiveChangesType?: DestructiveChangesType; private pathContentMap = new Map(); + private calculatedContent: string[] = []; public constructor( props: ComponentProperties, @@ -146,15 +147,17 @@ export class SourceComponent implements MetadataComponent { } public walkContent(): string[] { - const sources: string[] = []; - if (this.content) { - for (const fsPath of this.walk(this.content)) { - if (fsPath !== this.xml) { - sources.push(fsPath); + if (!this.calculatedContent.length) { + if (this.content) { + for (const fsPath of this.walk(this.content)) { + if (fsPath !== this.xml) { + this.calculatedContent.push(fsPath); + } } } } - return sources; + + return this.calculatedContent; } /** * returns the children of a parent SourceComponent diff --git a/test/resolve/adapters/bundleSourceAdapter.test.ts b/test/resolve/adapters/bundleSourceAdapter.test.ts index c4db7d4e36..7620922a67 100644 --- a/test/resolve/adapters/bundleSourceAdapter.test.ts +++ b/test/resolve/adapters/bundleSourceAdapter.test.ts @@ -13,6 +13,12 @@ import { CONTENT_PATH as LWC_CONTENT_PATH } from '../../mock/type-constants/lwcB import { RegistryAccess } from '../../../src'; describe('BundleSourceAdapter with AuraBundle', () => { + beforeEach(() => { + // @ts-ignore reset cache + bundle.COMPONENT.calculatedContent = []; + // @ts-ignore reset cache + lwcBundle.COMPONENT.calculatedContent = []; + }); const registryAccess = new RegistryAccess(); const adapter = new BundleSourceAdapter(bundle.COMPONENT.type, registryAccess, undefined, bundle.COMPONENT.tree); From 8827da29d1efd6b578c6846acc6aa97e25a62dbf Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 17 Sep 2024 11:39:33 -0600 Subject: [PATCH 2/8] chore: fix warnings --- src/client/metadataApiDeploy.ts | 2 +- src/client/metadataApiRetrieve.ts | 1 + src/client/metadataTransfer.ts | 2 +- src/convert/metadataConverter.ts | 7 ++----- src/convert/streams.ts | 2 +- .../transformers/metadataTransformerFactory.ts | 4 ++-- src/registry/registryAccess.ts | 2 +- src/resolve/adapters/baseSourceAdapter.ts | 5 +++-- src/resolve/adapters/decomposedSourceAdapter.ts | 1 + src/resolve/adapters/mixedContentSourceAdapter.ts | 1 - src/resolve/adapters/sourceAdapterFactory.ts | 4 ++-- src/resolve/connectionResolver.ts | 4 ++-- src/resolve/manifestResolver.ts | 4 +--- src/resolve/sourceComponent.ts | 12 ++++++------ src/utils/path.ts | 6 +++--- 15 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/client/metadataApiDeploy.ts b/src/client/metadataApiDeploy.ts index 284a65f887..8deadaa9f1 100644 --- a/src/client/metadataApiDeploy.ts +++ b/src/client/metadataApiDeploy.ts @@ -152,7 +152,7 @@ export class MetadataApiDeploy extends MetadataTransfer< } const connection = await this.getConnection(); // Recasting to use the project's version of the type - return connection.metadata.checkDeployStatus(this.id, true) as unknown as MetadataApiDeployStatus; + return (await connection.metadata.checkDeployStatus(this.id, true)) as unknown as MetadataApiDeployStatus; } /** diff --git a/src/client/metadataApiRetrieve.ts b/src/client/metadataApiRetrieve.ts index 4f0cc48c97..6dd0e97a9a 100644 --- a/src/client/metadataApiRetrieve.ts +++ b/src/client/metadataApiRetrieve.ts @@ -41,6 +41,7 @@ export class RetrieveResult implements MetadataTransferResult { * @param response The metadata retrieve response from the server * @param components The ComponentSet of retrieved source components * @param localComponents The ComponentSet used to create the retrieve request + * @param partialDeleteFileResponses any partially deleted file responses */ public constructor( public readonly response: MetadataApiRetrieveStatus, diff --git a/src/client/metadataTransfer.ts b/src/client/metadataTransfer.ts index 72dd9c693c..c6e719f12d 100644 --- a/src/client/metadataTransfer.ts +++ b/src/client/metadataTransfer.ts @@ -46,7 +46,7 @@ export abstract class MetadataTransfer< private transferId: Options['id']; private event = new EventEmitter(); private usernameOrConnection: string | Connection; - private apiVersion?: string; + private readonly apiVersion?: string; public constructor({ usernameOrConnection, components, apiVersion, id }: Options) { this.usernameOrConnection = usernameOrConnection; diff --git a/src/convert/metadataConverter.ts b/src/convert/metadataConverter.ts index 48b9c9286a..7383166cbd 100644 --- a/src/convert/metadataConverter.ts +++ b/src/convert/metadataConverter.ts @@ -28,8 +28,7 @@ export class MetadataConverter { public static readonly DESTRUCTIVE_CHANGES_POST_XML_FILE = 'destructiveChangesPost.xml'; public static readonly DESTRUCTIVE_CHANGES_PRE_XML_FILE = 'destructiveChangesPre.xml'; public static readonly DEFAULT_PACKAGE_PREFIX = 'metadataPackage'; - - private registry: RegistryAccess; + private readonly registry: RegistryAccess; public constructor(registry = new RegistryAccess()) { this.registry = registry; @@ -42,9 +41,7 @@ export class MetadataConverter { ): Promise { try { const cs = comps instanceof ComponentSet ? comps : new ComponentSet(comps, this.registry); - const components = ( - (comps instanceof ComponentSet ? Array.from(comps.getSourceComponents()) : comps) as SourceComponent[] - ).filter((comp) => comp.type.isAddressable !== false); + const components = cs.getSourceComponents().filter((comp) => comp.type.isAddressable !== false); if (output.type !== 'merge' && output.packageName) { cs.fullName = output.packageName; diff --git a/src/convert/streams.ts b/src/convert/streams.ts index 35e9f9a8da..ab687d505f 100644 --- a/src/convert/streams.ts +++ b/src/convert/streams.ts @@ -118,7 +118,7 @@ export abstract class ComponentWriter extends Writable { protected rootDestination?: SourcePath; protected logger: Logger; - public constructor(rootDestination?: SourcePath) { + protected constructor(rootDestination?: SourcePath) { super({ objectMode: true }); this.rootDestination = rootDestination; this.logger = Logger.childFromRoot(this.constructor.name); diff --git a/src/convert/transformers/metadataTransformerFactory.ts b/src/convert/transformers/metadataTransformerFactory.ts index 026ca3142a..46423579fd 100644 --- a/src/convert/transformers/metadataTransformerFactory.ts +++ b/src/convert/transformers/metadataTransformerFactory.ts @@ -20,8 +20,8 @@ Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr'); export class MetadataTransformerFactory { - private registry: RegistryAccess; - private context: ConvertContext; + private readonly registry: RegistryAccess; + private readonly context: ConvertContext; public constructor(registry: RegistryAccess, context = new ConvertContext()) { this.registry = registry; diff --git a/src/registry/registryAccess.ts b/src/registry/registryAccess.ts index aa8ba7f6ce..8cc348d9a8 100644 --- a/src/registry/registryAccess.ts +++ b/src/registry/registryAccess.ts @@ -17,7 +17,7 @@ Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr'); export class RegistryAccess { - private registry: MetadataRegistry; + private readonly registry: MetadataRegistry; private strictFolderTypes?: MetadataType[]; private folderContentTypes?: MetadataType[]; private aliasTypes?: MetadataType[]; diff --git a/src/resolve/adapters/baseSourceAdapter.ts b/src/resolve/adapters/baseSourceAdapter.ts index 2e232a1a9b..c658c523b2 100644 --- a/src/resolve/adapters/baseSourceAdapter.ts +++ b/src/resolve/adapters/baseSourceAdapter.ts @@ -94,7 +94,7 @@ export abstract class BaseSourceAdapter implements SourceAdapter { protected parseAsRootMetadataXml(path: SourcePath): MetadataXml | undefined { const metaXml = this.parseMetadataXml(path); if (metaXml) { - let isRootMetadataXml = false; + let isRootMetadataXml: boolean; if (this.type.strictDirectoryName) { const parentPath = dirname(path); const typeDirName = basename(this.type.inFolder ? dirname(parentPath) : parentPath); @@ -138,6 +138,7 @@ export abstract class BaseSourceAdapter implements SourceAdapter { * * @param component Component to populate properties on * @param trigger Path that `getComponent` was called with + * @param isResolvingSource if you're resolving a local source file */ protected abstract populate( trigger: SourcePath, @@ -153,7 +154,7 @@ export abstract class BaseSourceAdapter implements SourceAdapter { * * .../tabs/MyTab.tab * - * @param path File path of a metadata component + * @param type */ const parseAsContentMetadataXml = (type: MetadataType) => diff --git a/src/resolve/adapters/decomposedSourceAdapter.ts b/src/resolve/adapters/decomposedSourceAdapter.ts index aef8af0ad4..a32d648a63 100644 --- a/src/resolve/adapters/decomposedSourceAdapter.ts +++ b/src/resolve/adapters/decomposedSourceAdapter.ts @@ -92,6 +92,7 @@ export class DecomposedSourceAdapter extends MixedContentSourceAdapter { triggerIsAChild && this.type.children && !this.type.children.types[childTypeId].unaddressableWithoutParent && + // make sure isAddressable is truly set to 'false' not just undefined or omitted this.type.children.types[childTypeId].isAddressable !== false ) { if (strategy === 'folderPerType' || strategy === 'topLevel' || isResolvingSource) { diff --git a/src/resolve/adapters/mixedContentSourceAdapter.ts b/src/resolve/adapters/mixedContentSourceAdapter.ts index bdada55421..ce204c701b 100644 --- a/src/resolve/adapters/mixedContentSourceAdapter.ts +++ b/src/resolve/adapters/mixedContentSourceAdapter.ts @@ -91,7 +91,6 @@ export class MixedContentSourceAdapter extends BaseSourceAdapter { * folder will be returned. Intended to be used exclusively for MixedContent types. * * @param path Path to trim - * @param type MetadataType to determine content for */ protected trimPathToContent(path: SourcePath): SourcePath { const pathParts = path.split(sep); diff --git a/src/resolve/adapters/sourceAdapterFactory.ts b/src/resolve/adapters/sourceAdapterFactory.ts index d1ab2d0e0a..4f5d707808 100644 --- a/src/resolve/adapters/sourceAdapterFactory.ts +++ b/src/resolve/adapters/sourceAdapterFactory.ts @@ -21,8 +21,8 @@ Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr'); export class SourceAdapterFactory { - private registry: RegistryAccess; - private tree: TreeContainer; + private readonly registry: RegistryAccess; + private readonly tree: TreeContainer; public constructor(registry: RegistryAccess, tree: TreeContainer) { this.registry = registry; diff --git a/src/resolve/connectionResolver.ts b/src/resolve/connectionResolver.ts index 3109b58807..b206081c6e 100644 --- a/src/resolve/connectionResolver.ts +++ b/src/resolve/connectionResolver.ts @@ -28,8 +28,8 @@ export type ResolveConnectionResult = { * Resolve MetadataComponents from an org connection */ export class ConnectionResolver { - private connection: Connection; - private registry: RegistryAccess; + private readonly connection: Connection; + private readonly registry: RegistryAccess; // Array of metadata type names to use for listMembers. By default it includes // all types defined in the registry. diff --git a/src/resolve/manifestResolver.ts b/src/resolve/manifestResolver.ts index 5967d7ca81..39b9b89c3a 100644 --- a/src/resolve/manifestResolver.ts +++ b/src/resolve/manifestResolver.ts @@ -61,9 +61,7 @@ export class ManifestResolver { numberParseOptions: { leadingZeros: false, hex: false, skipLike: /\.0$/ }, }); - const parsedManifest: ParsedPackageManifest = ( - parser.parse(validatedContents) as { Package: ParsedPackageManifest } - ).Package; + const parsedManifest = (parser.parse(validatedContents) as { Package: ParsedPackageManifest }).Package; const components = ensureArray(parsedManifest.types) .map(getValidatedType(manifestPath)) diff --git a/src/resolve/sourceComponent.ts b/src/resolve/sourceComponent.ts index c67ca7e293..34ce28f86d 100644 --- a/src/resolve/sourceComponent.ts +++ b/src/resolve/sourceComponent.ts @@ -393,12 +393,12 @@ export class SourceComponent implements MetadataComponent { } else { for (const child of this.treeContainer.readDirectory(fsPath)) { const childPath = join(fsPath, child); - if (this.forceIgnore.denies(childPath)) { - continue; - } else if (this.treeContainer.isDirectory(childPath)) { - yield* this.walk(childPath); - } else { - yield childPath; + if (!this.forceIgnore.denies(childPath)) { + if (this.treeContainer.isDirectory(childPath)) { + yield* this.walk(childPath); + } else { + yield childPath; + } } } } diff --git a/src/utils/path.ts b/src/utils/path.ts index 69312c130d..33181dff1e 100644 --- a/src/utils/path.ts +++ b/src/utils/path.ts @@ -5,7 +5,7 @@ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ -import { basename, dirname, extname, sep, join } from 'node:path'; +import { basename, dirname, extname, join, sep } from 'node:path'; import { Optional } from '@salesforce/ts-types'; import { SfdxFileFormat } from '../convert/types'; import { SourcePath } from '../common/types'; @@ -28,6 +28,7 @@ export function baseName(fsPath: SourcePath): string { * this will handle that, but requires you to specify the mdType to check suffixes for. * * @param fsPath The path to evaluate + * @param mdType the metadata type to check suffixes of */ export function baseWithoutSuffixes(fsPath: SourcePath, mdType: MetadataType): string { return basename(fsPath).replace(META_XML_SUFFIX, '').split('.').filter(stringIsNotSuffix(mdType)).join('.'); @@ -118,8 +119,7 @@ export function parseNestedFullName(fsPath: string, directoryName: string): stri const pathPrefix = pathSplits.slice(pathSplits.lastIndexOf(directoryName) + 1); // the eslint comment should remain until strictMode is fully implemented // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - const fileName = (pathSplits.pop() as string).replace('-meta.xml', '').split('.')[0]; - pathPrefix[pathPrefix.length - 1] = fileName; + pathPrefix[pathPrefix.length - 1] = (pathSplits.pop() as string).replace('-meta.xml', '').split('.')[0]; return pathPrefix.join('/'); } From 5e08eb4f8f8458146a1b3c1b82cd241a567a2aa7 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 17 Sep 2024 11:51:51 -0600 Subject: [PATCH 3/8] chore: fix lint warnings --- src/client/metadataApiDeploy.ts | 2 +- src/convert/replacements.ts | 11 ++++++----- src/resolve/adapters/decomposedSourceAdapter.ts | 12 +++++++----- src/resolve/adapters/matchingContentSourceAdapter.ts | 7 ++++--- src/resolve/adapters/mixedContentSourceAdapter.ts | 10 ++++++---- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/client/metadataApiDeploy.ts b/src/client/metadataApiDeploy.ts index 8deadaa9f1..d06c4b7866 100644 --- a/src/client/metadataApiDeploy.ts +++ b/src/client/metadataApiDeploy.ts @@ -103,8 +103,8 @@ export class MetadataApiDeploy extends MetadataTransfer< public constructor(options: MetadataApiDeployOptions) { super(options); - options.apiOptions = { ...MetadataApiDeploy.DEFAULT_OPTIONS.apiOptions, ...options.apiOptions }; this.options = Object.assign({}, options); + this.options.apiOptions = { ...MetadataApiDeploy.DEFAULT_OPTIONS.apiOptions, ...options.apiOptions }; this.isRestDeploy = !!options.apiOptions?.rest; this.registry = options.registry ?? new RegistryAccess(); if (this.mdapiTempDir) { diff --git a/src/convert/replacements.ts b/src/convert/replacements.ts index e583b32419..02fc63df3c 100644 --- a/src/convert/replacements.ts +++ b/src/convert/replacements.ts @@ -117,16 +117,17 @@ class ReplacementMarkingStream extends Transform { encoding: string, callback: (err: Error | undefined, data: SourceComponent) => void ): Promise { + const toBeReturned = chunk; let err: Error | undefined; // if deleting, or no configs, just pass through - if (!chunk.isMarkedForDelete() && this.replacementConfigs?.length) { + if (!toBeReturned.isMarkedForDelete() && this.replacementConfigs?.length) { try { - chunk.replacements = await getReplacements(chunk, this.replacementConfigs); - if (chunk.replacements && chunk.parent?.type.strategies?.transformer === 'nonDecomposed') { + toBeReturned.replacements = await getReplacements(toBeReturned, this.replacementConfigs); + if (toBeReturned.replacements && toBeReturned.parent?.type.strategies?.transformer === 'nonDecomposed') { // Set replacements on the parent of a nonDecomposed CustomLabel as well so that recomposing // doesn't use the non-replaced content from parent cache. // See RecompositionFinalizer.recompose() in convertContext.ts - chunk.parent.replacements = chunk.replacements; + toBeReturned.parent.replacements = toBeReturned.replacements; } } catch (e) { if (!(e instanceof Error)) { @@ -135,7 +136,7 @@ class ReplacementMarkingStream extends Transform { err = e; } } - callback(err, chunk); + callback(err, toBeReturned); } } diff --git a/src/resolve/adapters/decomposedSourceAdapter.ts b/src/resolve/adapters/decomposedSourceAdapter.ts index a32d648a63..e130419fd4 100644 --- a/src/resolve/adapters/decomposedSourceAdapter.ts +++ b/src/resolve/adapters/decomposedSourceAdapter.ts @@ -83,6 +83,8 @@ export class DecomposedSourceAdapter extends MixedContentSourceAdapter { isResolvingSource?: boolean ): SourceComponent | undefined { const metaXml = parseMetadataXml(trigger); + const toBeReturned = component; + if (metaXml?.suffix) { const pathToContent = this.trimPathToContent(trigger); const childTypeId = this.type.children?.suffixes?.[metaXml.suffix]; @@ -97,7 +99,7 @@ export class DecomposedSourceAdapter extends MixedContentSourceAdapter { ) { if (strategy === 'folderPerType' || strategy === 'topLevel' || isResolvingSource) { const parent = - component ?? + toBeReturned ?? new SourceComponent( { name: strategy === 'folderPerType' ? baseName(pathToContent) : pathToContent, @@ -118,7 +120,7 @@ export class DecomposedSourceAdapter extends MixedContentSourceAdapter { this.forceIgnore ); } - } else if (!component) { + } else if (!toBeReturned) { // This is most likely metadata found within a CustomObject folder that is not a // child type of CustomObject. E.g., Layout, SharingRules, ApexClass. throw new SfError( @@ -126,10 +128,10 @@ export class DecomposedSourceAdapter extends MixedContentSourceAdapter { 'TypeInferenceError' ); } - if (component) { - component.content = pathToContent; + if (toBeReturned) { + toBeReturned.content = pathToContent; } } - return component; + return toBeReturned; } } diff --git a/src/resolve/adapters/matchingContentSourceAdapter.ts b/src/resolve/adapters/matchingContentSourceAdapter.ts index 6445d9baf0..45d478867f 100644 --- a/src/resolve/adapters/matchingContentSourceAdapter.ts +++ b/src/resolve/adapters/matchingContentSourceAdapter.ts @@ -39,8 +39,9 @@ export class MatchingContentSourceAdapter extends BaseSourceAdapter { protected populate(trigger: SourcePath, component: SourceComponent): SourceComponent { let sourcePath: SourcePath | undefined; + const toBeReturned = component; - if (component.xml === trigger) { + if (toBeReturned.xml === trigger) { const fsPath = removeMetaXmlSuffix(trigger); if (this.tree.exists(fsPath)) { sourcePath = fsPath; @@ -58,8 +59,8 @@ export class MatchingContentSourceAdapter extends BaseSourceAdapter { throw messages.createError('noSourceIgnore', [this.type.name, sourcePath]); } - component.content = sourcePath; - return component; + toBeReturned.content = sourcePath; + return toBeReturned; } } diff --git a/src/resolve/adapters/mixedContentSourceAdapter.ts b/src/resolve/adapters/mixedContentSourceAdapter.ts index ce204c701b..2499fcb5f7 100644 --- a/src/resolve/adapters/mixedContentSourceAdapter.ts +++ b/src/resolve/adapters/mixedContentSourceAdapter.ts @@ -67,10 +67,12 @@ export class MixedContentSourceAdapter extends BaseSourceAdapter { ); } - if (component) { - component.content = contentPath; + let toBeReturned = component; + + if (toBeReturned) { + toBeReturned.content = contentPath; } else { - component = new SourceComponent( + toBeReturned = new SourceComponent( { name: baseName(contentPath), type: this.type, @@ -82,7 +84,7 @@ export class MixedContentSourceAdapter extends BaseSourceAdapter { ); } - return component; + return toBeReturned; } /** From 42a9c8dda0e68f636dbcf0adccee937748de522e Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 17 Sep 2024 14:02:08 -0600 Subject: [PATCH 4/8] chore: remove TODOs in forceignore --- src/convert/replacements.ts | 5 +---- src/resolve/forceIgnore.ts | 4 ++-- test/resolve/forceIgnore.test.ts | 33 +++++++++----------------------- 3 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/convert/replacements.ts b/src/convert/replacements.ts index 02fc63df3c..83392c8533 100644 --- a/src/convert/replacements.ts +++ b/src/convert/replacements.ts @@ -66,10 +66,7 @@ export const replacementIterations = async (input: string, replacements: MarkedR const lifecycleInstance = Lifecycle.getInstance(); let output = input; for (const replacement of replacements) { - // TODO: node 16+ has String.replaceAll for non-regex scenarios - const regex = - typeof replacement.toReplace === 'string' ? new RegExp(replacement.toReplace, 'g') : replacement.toReplace; - const replaced = output.replace(regex, replacement.replaceWith ?? ''); + const replaced = output.replaceAll(new RegExp(replacement.toReplace, 'g'), replacement.replaceWith ?? ''); if (replaced !== output) { output = replaced; diff --git a/src/resolve/forceIgnore.ts b/src/resolve/forceIgnore.ts index 66468071a1..baa99bbde8 100644 --- a/src/resolve/forceIgnore.ts +++ b/src/resolve/forceIgnore.ts @@ -8,7 +8,7 @@ import { dirname, join, relative } from 'node:path'; import ignore, { Ignore } from 'ignore/index'; import { readFileSync } from 'graceful-fs'; -import { Lifecycle } from '@salesforce/core'; +import { Lifecycle, Logger } from '@salesforce/core'; import { SourcePath } from '../common/types'; import { searchUp } from '../utils/fileSystemHandler'; @@ -42,7 +42,7 @@ export class ForceIgnore { this.forceIgnoreDirectory = dirname(forceIgnorePath); } } catch (e) { - // TODO: log no force ignore + Logger.childFromRoot(this.constructor.name).info('no .forceignore found'); } } diff --git a/test/resolve/forceIgnore.test.ts b/test/resolve/forceIgnore.test.ts index bb3b52dcec..007ffbd246 100644 --- a/test/resolve/forceIgnore.test.ts +++ b/test/resolve/forceIgnore.test.ts @@ -84,30 +84,15 @@ describe('ForceIgnore', () => { forceIgnore = new ForceIgnore(); }); - it('Should ignore files starting with a dot', () => { - const dotPath = join(root, '.xyz'); - - expect(forceIgnore.accepts(dotPath)).to.be.false; - expect(forceIgnore.denies(dotPath)).to.be.true; - }); - - it('Should ignore files ending in .dup', () => { - const dupPath = join(root, 'abc.dup'); - - expect(forceIgnore.accepts(dupPath)).to.be.false; - expect(forceIgnore.denies(dupPath)).to.be.true; - }); - - it('Should ignore files named package2-descriptor.json', () => { - const descriptorPath = join(root, 'package2-descriptor.json'); - expect(forceIgnore.accepts(descriptorPath)).to.be.false; - expect(forceIgnore.denies(descriptorPath)).to.be.true; - }); - - it('Should ignore files named package2-manifest.json', () => { - const manifestPath = join(root, 'package2-manifest.json'); - expect(forceIgnore.accepts(manifestPath)).to.be.false; - expect(forceIgnore.denies(manifestPath)).to.be.true; + // the example's index here is specific to the rules order in ForceIgnore.DEFAULT_IGNORE + const forceIgnoreExamples = ['abc.dup', '.xyz', 'package2-descriptor.json', 'package2-manifest.json']; + forceIgnoreExamples.map((ignore) => { + it(`Should ignore files starting with a ${ignore}`, () => { + const testPath = join(root, ignore); + + expect(forceIgnore.accepts(testPath)).to.be.false; + expect(forceIgnore.denies(testPath)).to.be.true; + }); }); it('Should allow .forceignore file to override defaults', () => { From ebdff159c48a5c422f4305f36f2c36f7ba5c06ad Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Tue, 17 Sep 2024 14:56:12 -0600 Subject: [PATCH 5/8] chore: revert caching to see if it fixes NUT --- src/resolve/sourceComponent.ts | 15 ++++++--------- test/resolve/adapters/bundleSourceAdapter.test.ts | 6 ------ 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/resolve/sourceComponent.ts b/src/resolve/sourceComponent.ts index 34ce28f86d..c4e059e338 100644 --- a/src/resolve/sourceComponent.ts +++ b/src/resolve/sourceComponent.ts @@ -54,7 +54,6 @@ export class SourceComponent implements MetadataComponent { private markedForDelete = false; private destructiveChangesType?: DestructiveChangesType; private pathContentMap = new Map(); - private calculatedContent: string[] = []; public constructor( props: ComponentProperties, @@ -147,17 +146,15 @@ export class SourceComponent implements MetadataComponent { } public walkContent(): string[] { - if (!this.calculatedContent.length) { - if (this.content) { - for (const fsPath of this.walk(this.content)) { - if (fsPath !== this.xml) { - this.calculatedContent.push(fsPath); - } + const sources: string[] = []; + if (this.content) { + for (const fsPath of this.walk(this.content)) { + if (fsPath !== this.xml) { + sources.push(fsPath); } } } - - return this.calculatedContent; + return sources; } /** * returns the children of a parent SourceComponent diff --git a/test/resolve/adapters/bundleSourceAdapter.test.ts b/test/resolve/adapters/bundleSourceAdapter.test.ts index 7620922a67..c4db7d4e36 100644 --- a/test/resolve/adapters/bundleSourceAdapter.test.ts +++ b/test/resolve/adapters/bundleSourceAdapter.test.ts @@ -13,12 +13,6 @@ import { CONTENT_PATH as LWC_CONTENT_PATH } from '../../mock/type-constants/lwcB import { RegistryAccess } from '../../../src'; describe('BundleSourceAdapter with AuraBundle', () => { - beforeEach(() => { - // @ts-ignore reset cache - bundle.COMPONENT.calculatedContent = []; - // @ts-ignore reset cache - lwcBundle.COMPONENT.calculatedContent = []; - }); const registryAccess = new RegistryAccess(); const adapter = new BundleSourceAdapter(bundle.COMPONENT.type, registryAccess, undefined, bundle.COMPONENT.tree); From 969c2e4ed344a115de711cd087539e9aa2b3c795 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 18 Sep 2024 10:26:32 -0600 Subject: [PATCH 6/8] chore: forceignore caching --- src/resolve/forceIgnore.ts | 25 +++++++++++++++++++++---- src/resolve/sourceComponent.ts | 2 +- src/resolve/treeContainers.ts | 8 +++++++- test/resolve/forceIgnore.test.ts | 8 ++------ 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/resolve/forceIgnore.ts b/src/resolve/forceIgnore.ts index baa99bbde8..b154301677 100644 --- a/src/resolve/forceIgnore.ts +++ b/src/resolve/forceIgnore.ts @@ -18,6 +18,7 @@ export class ForceIgnore { private readonly parser?: Ignore; private readonly forceIgnoreDirectory?: string; private DEFAULT_IGNORE = ['**/*.dup', '**/.*', '**/package2-descriptor.json', '**/package2-manifest.json']; + private isPathIgnored = new Map(); public constructor(forceIgnorePath = '') { try { @@ -64,19 +65,35 @@ export class ForceIgnore { public denies(fsPath: SourcePath): boolean { if (!this.parser || !this.forceIgnoreDirectory) return false; + // we've already figured out if this path is ignored or not, just get it from the cache + if (this.isPathIgnored.has(fsPath)) return this.isPathIgnored.get(fsPath)!; + + let result: boolean; try { - return this.parser.ignores(relative(this.forceIgnoreDirectory, fsPath)); + result = this.parser.ignores(relative(this.forceIgnoreDirectory, fsPath)); } catch (e) { - return false; + result = false; } + + this.isPathIgnored.set(fsPath, result); + + return result; } public accepts(fsPath: SourcePath): boolean { if (!this.parser || !this.forceIgnoreDirectory) return true; + // we've already figured out if this path is ignored or not, just get it from the cache + // the cache is set for 'denies' so for accept, negate the result + if (this.isPathIgnored.has(fsPath)) return !this.isPathIgnored.get(fsPath); + + let result: boolean; try { - return !this.parser.ignores(relative(this.forceIgnoreDirectory, fsPath)); + result = !this.parser.ignores(relative(this.forceIgnoreDirectory, fsPath)); } catch (e) { - return true; + result = true; } + // since the cache has the 'denies' result, negate the result here + this.isPathIgnored.set(fsPath, !result); + return result; } } diff --git a/src/resolve/sourceComponent.ts b/src/resolve/sourceComponent.ts index c4e059e338..7abb3e8a3c 100644 --- a/src/resolve/sourceComponent.ts +++ b/src/resolve/sourceComponent.ts @@ -287,7 +287,7 @@ export class SourceComponent implements MetadataComponent { } private parse(contents: string): T { - const parsed = parser.parse(String(contents)) as T; + const parsed = parser.parse(contents) as T; const [firstElement] = Object.keys(parsed); if (firstElement === this.type.name) { return parsed; diff --git a/src/resolve/treeContainers.ts b/src/resolve/treeContainers.ts index 6462acd217..eacefa46d6 100644 --- a/src/resolve/treeContainers.ts +++ b/src/resolve/treeContainers.ts @@ -25,6 +25,7 @@ const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sd * Extend this base class to implement a custom container. */ export abstract class TreeContainer { + protected fileContentMap: Map = new Map(); /** * Searches for a metadata component file in a container directory. * @@ -110,7 +111,12 @@ export class NodeFSTreeContainer extends TreeContainer { } public readFileSync(fsPath: SourcePath): Buffer { - return readFileSync(fsPath); + if (this.fileContentMap.has(fsPath)) { + return this.fileContentMap.get(fsPath)!; + } else { + this.fileContentMap.set(fsPath, readFileSync(fsPath)); + return this.fileContentMap.get(fsPath)!; + } } public stream(fsPath: SourcePath): Readable { diff --git a/test/resolve/forceIgnore.test.ts b/test/resolve/forceIgnore.test.ts index 007ffbd246..c341e6ddd7 100644 --- a/test/resolve/forceIgnore.test.ts +++ b/test/resolve/forceIgnore.test.ts @@ -71,10 +71,6 @@ describe('ForceIgnore', () => { expect(fi.accepts(join('force-app', 'main', 'default', 'classes'))).to.be.true; }); - /** - * TODO: Rework when approach to default patterns changes. We should be able - * to generally test the defaults system. - */ describe('Defaults with new parser', () => { let forceIgnore: ForceIgnore; const root = join('some', 'path'); @@ -84,8 +80,8 @@ describe('ForceIgnore', () => { forceIgnore = new ForceIgnore(); }); - // the example's index here is specific to the rules order in ForceIgnore.DEFAULT_IGNORE - const forceIgnoreExamples = ['abc.dup', '.xyz', 'package2-descriptor.json', 'package2-manifest.json']; + // these examples test the default behaviors - check the cache behavior with the duplicate 'abc.dup' + const forceIgnoreExamples = ['abc.dup', 'abc.dup', '.xyz', 'package2-descriptor.json', 'package2-manifest.json']; forceIgnoreExamples.map((ignore) => { it(`Should ignore files starting with a ${ignore}`, () => { const testPath = join(root, ignore); From 9b864fc7f0c91fe83dda1cb4a3affd484e0ce65f Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 18 Sep 2024 11:50:04 -0600 Subject: [PATCH 7/8] chore: cache NodeFSTree read --- src/resolve/treeContainers.ts | 10 ++++++++-- test/resolve/treeContainers.test.ts | 15 +++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/resolve/treeContainers.ts b/src/resolve/treeContainers.ts index eacefa46d6..38a470bfca 100644 --- a/src/resolve/treeContainers.ts +++ b/src/resolve/treeContainers.ts @@ -106,8 +106,14 @@ export class NodeFSTreeContainer extends TreeContainer { } public readFile(fsPath: SourcePath): Promise { - // significant enough performance increase using sync instead of fs.promise version - return Promise.resolve(readFileSync(fsPath)); + if (this.fileContentMap.has(fsPath)) { + return Promise.resolve(this.fileContentMap.get(fsPath)!); + } else { + // significant enough performance increase using sync instead of fs.promise version + const content = readFileSync(fsPath); + this.fileContentMap.set(fsPath, content); + return Promise.resolve(content); + } } public readFileSync(fsPath: SourcePath): Buffer { diff --git a/test/resolve/treeContainers.test.ts b/test/resolve/treeContainers.test.ts index 0bb7af61e9..e7136ba791 100644 --- a/test/resolve/treeContainers.test.ts +++ b/test/resolve/treeContainers.test.ts @@ -119,12 +119,23 @@ describe('Tree Containers', () => { it('should use expected Node API for readFileSync', () => { const readFileStub = env.stub(fs, 'readFileSync'); // @ts-ignore wants Dirents but string[] works as well - readFileStub.withArgs(path).returns(Buffer.from('test')); - const data = tree.readFileSync(path); + readFileStub.withArgs('myNewPath').returns(Buffer.from('test')); + const data = tree.readFileSync('myNewPath'); expect(data.toString()).to.deep.equal('test'); expect(readFileStub.calledOnce).to.be.true; }); + it('should use cached value for readFileSync', () => { + const readFileStub = env.stub(fs, 'readFileSync'); + // @ts-ignore wants Dirents but string[] works as well + readFileStub.withArgs('myNewPath').returns(Buffer.from('test')); + const data = tree.readFileSync('myNewPath'); + // returns same value + expect(data.toString()).to.deep.equal('test'); + // didn't re-read the file + expect(readFileStub.called).to.be.false; + }); + it('should use expected Node API for stream', () => { const readable = new Readable(); // @ts-ignore wants ReadStream but Readable works for testing From 407d4253aa38be0fe606dec723dbd63e7b5b2617 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Wed, 18 Sep 2024 13:53:07 -0600 Subject: [PATCH 8/8] chore: remove a few eslint issues --- src/convert/transformers/decomposeLabelsTransformer.ts | 3 ++- src/resolve/metadataResolver.ts | 2 +- src/resolve/treeContainers.ts | 7 +++---- src/utils/metadata.ts | 4 ++-- src/utils/path.ts | 4 +--- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/convert/transformers/decomposeLabelsTransformer.ts b/src/convert/transformers/decomposeLabelsTransformer.ts index ed6079280e..a9b4cb3872 100644 --- a/src/convert/transformers/decomposeLabelsTransformer.ts +++ b/src/convert/transformers/decomposeLabelsTransformer.ts @@ -13,6 +13,7 @@ import { SourceComponent } from '../../resolve/sourceComponent'; import { ToSourceFormatInput, WriteInfo } from '../types'; import { JsToXml } from '../streams'; import { unwrapAndOmitNS } from '../../utils/decomposed'; +import { META_XML_SUFFIX } from '../../common'; import { DefaultMetadataTransformer } from './defaultMetadataTransformer'; /* Use for the metadata type CustomLabels */ @@ -31,7 +32,7 @@ export class LabelsMetadataTransformer extends DefaultMetadataTransformer { output: // if present in the merge set, use that xml path, otherwise use the default path mergeSet?.getComponentFilenamesByNameAndType({ fullName: l.fullName, type: labelType.name })?.[0] ?? - partiallyAppliedPathCalculator(l.fullName)(`${l.fullName}.label-meta.xml`), + partiallyAppliedPathCalculator(l.fullName)(`${l.fullName}.label${META_XML_SUFFIX}`), source: new JsToXml({ CustomLabel: l }), })); } diff --git a/src/resolve/metadataResolver.ts b/src/resolve/metadataResolver.ts index a70dabc4c3..f5d852f37d 100644 --- a/src/resolve/metadataResolver.ts +++ b/src/resolve/metadataResolver.ts @@ -271,7 +271,7 @@ const getSuggestionsForUnresolvedTypes = ...guesses.map((guess) => messages.getMessage('suggest_type_did_you_mean', [ guess.suffixGuess, - typeof metaSuffix === 'string' || closeMetaSuffix ? '-meta.xml' : '', + typeof metaSuffix === 'string' || closeMetaSuffix ? META_XML_SUFFIX : '', guess.metadataTypeGuess.name, ]) ), diff --git a/src/resolve/treeContainers.ts b/src/resolve/treeContainers.ts index 38a470bfca..5dfdb2fe4a 100644 --- a/src/resolve/treeContainers.ts +++ b/src/resolve/treeContainers.ts @@ -237,7 +237,6 @@ export class ZipTreeContainer extends TreeContainer { */ export class VirtualTreeContainer extends TreeContainer { private tree = new Map>(); - private fileContents = new Map(); public constructor(virtualFs: VirtualDirectory[]) { super(); @@ -298,10 +297,10 @@ export class VirtualTreeContainer extends TreeContainer { public readFileSync(fsPath: SourcePath): Buffer { if (this.exists(fsPath)) { - let data = this.fileContents.get(fsPath); + let data = this.fileContentMap.get(fsPath); if (!data) { data = Buffer.from(''); - this.fileContents.set(fsPath, data); + this.fileContentMap.set(fsPath, data); } return data; } @@ -327,7 +326,7 @@ export class VirtualTreeContainer extends TreeContainer { dirPathFromTree.add(childPath); if (typeof child === 'object' && child.data) { - this.fileContents.set(childPath, child.data); + this.fileContentMap.set(childPath, child.data); } } } diff --git a/src/utils/metadata.ts b/src/utils/metadata.ts index d09c72aadc..6a89e45f93 100644 --- a/src/utils/metadata.ts +++ b/src/utils/metadata.ts @@ -8,7 +8,7 @@ import type { CustomLabel } from '@jsforce/jsforce-node/lib/api/metadata'; import { SfError } from '@salesforce/core'; import { XMLParser } from 'fast-xml-parser'; -import { META_XML_SUFFIX, XML_COMMENT_PROP_NAME } from '../common/constants'; +import { META_XML_SUFFIX, XML_COMMENT_PROP_NAME, XML_DECL } from '../common/constants'; export const parser = new XMLParser({ // include tag attributes and don't parse text node as number @@ -22,7 +22,7 @@ export const parser = new XMLParser({ }); export function generateMetaXML(typeName: string, apiVersion: string, status: string): string { - let templateResult = '\n'; + let templateResult = XML_DECL; templateResult += `<${typeName} xmlns="http://soap.sforce.com/2006/04/metadata">\n`; templateResult += `\t${apiVersion}.0\n`; if (status) { diff --git a/src/utils/path.ts b/src/utils/path.ts index 33181dff1e..82a865d1f0 100644 --- a/src/utils/path.ts +++ b/src/utils/path.ts @@ -117,9 +117,7 @@ export function parseNestedFullName(fsPath: string, directoryName: string): stri return; } const pathPrefix = pathSplits.slice(pathSplits.lastIndexOf(directoryName) + 1); - // the eslint comment should remain until strictMode is fully implemented - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion - pathPrefix[pathPrefix.length - 1] = (pathSplits.pop() as string).replace('-meta.xml', '').split('.')[0]; + pathPrefix[pathPrefix.length - 1] = (pathSplits.pop() as string).replace(META_XML_SUFFIX, '').split('.')[0]; return pathPrefix.join('/'); }