Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: no nullish in template literal #627

Merged
merged 3 commits into from
Jul 30, 2024
Merged

fix: no nullish in template literal #627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cdd6fae
chore: updates from devScripts
svc-cli-bot Jul 28, 2024
1814bc5
fix(deps): open dep updates
mshanemc Jul 30, 2024
f2a01d7
fix: linter updates for nulls in template literals
mshanemc Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@
],
"dependencies": {
"@jsforce/jsforce-node": "^3.2.0",
"@salesforce/core": "^8.2.3",
"@salesforce/core": "^8.2.7",
"@salesforce/kit": "^3.1.6",
"@salesforce/schemas": "^1.9.0",
"@salesforce/source-deploy-retrieve": "^12.1.8",
"@salesforce/ts-types": "^2.0.10",
"@salesforce/ts-types": "^2.0.11",
"@salesforce/types": "^1.2.0",
"fast-xml-parser": "^4.4.0",
"fast-xml-parser": "^4.4.1",
"globby": "^11",
"graphology": "^0.25.4",
"graphology-traversal": "^0.3.1",
Expand All @@ -57,14 +57,14 @@
"object-treeify": "^2"
},
"devDependencies": {
"@salesforce/cli-plugins-testkit": "^5.3.18",
"@salesforce/dev-scripts": "^10.2.2",
"@salesforce/cli-plugins-testkit": "^5.3.20",
"@salesforce/dev-scripts": "^10.2.8",
"@types/globby": "^9.1.0",
"@types/jszip": "^3.4.1",
"eslint-plugin-sf-plugin": "^1.18.11",
"eslint-plugin-sf-plugin": "^1.20.1",
"shelljs": "0.8.5",
"ts-node": "^10.9.2",
"typescript": "^5.5.3"
"typescript": "^5.5.4"
},
"publishConfig": {
"access": "public"
Expand Down
4 changes: 3 additions & 1 deletion src/package/packageAncestry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,9 @@ export class PackageAncestry extends AsyncCreatable<PackageAncestryOptions> {
// Check to see if the package is an unlocked package
// if so, throw and error since ancestry only applies to managed packages
await this.validatePackageType();
const normalQuery = `${SELECT_PACKAGE_VERSION} WHERE AncestorId = NULL AND Package2Id = '${this.requestedPackageId}' ${releasedOnlyFilter}`;
const normalQuery = `${SELECT_PACKAGE_VERSION} WHERE AncestorId = NULL AND Package2Id = '${
this.requestedPackageId ?? ''
Copy link
Contributor

@mshanemc mshanemc Jul 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where queries and consts are being built, I preserved the existing behavior (with `?? '') to let the server handle the missing value like it currently would

}' ${releasedOnlyFilter}`;
const subscriberPackageVersions = (
await this.options.connection.tooling.query<PackageAncestryNode>(normalQuery)
).records?.map((record) => new PackageAncestryNode(record));
Expand Down
8 changes: 5 additions & 3 deletions src/package/packageConvert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,19 +205,21 @@ export async function createPackageVersionCreateRequest(
if (hasSeedMetadata) {
// Zip the seedMetadataFolder folder and put the zip in {packageVersBlobDirectory}/{seedMetadataZipFile}
Logger.childFromRoot('packageConvert:pollForStatusWithInterval').debug(
`Including metadata found in '${context.seedmetadata}'.`
`Including metadata found in '${context.seedmetadata ?? '<undefined seedmetadata>'}'.`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logs get a more descriptive indicator of what's missing from where

);
await pkgUtils.zipDir(seedMetadataFolder, seedMetadataZipFile);
}

await settingsGenerator.createDeploy();
await settingsGenerator.createDeployPackageContents(apiVersion);
await pkgUtils.zipDir(
`${settingsGenerator.getDestinationPath()}${path.sep}${settingsGenerator.getShapeDirName()}`,
`${settingsGenerator.getDestinationPath() ?? ''}${path.sep}${settingsGenerator.getShapeDirName()}`,
settingsZipFile
);

const shapeDirectory = `${settingsGenerator.getDestinationPath()}${path.sep}${settingsGenerator.getShapeDirName()}`;
const shapeDirectory = `${settingsGenerator.getDestinationPath() ?? ''}${
path.sep
}${settingsGenerator.getShapeDirName()}`;
const currentPackageXml = await fs.promises.readFile(path.join(shapeDirectory, 'package.xml'), 'utf8');
await fs.promises.writeFile(path.join(packageVersMetadataFolder, 'package.xml'), currentPackageXml, 'utf-8');
// Zip the packageVersMetadataFolder folder and put the zip in {packageVersBlobDirectory}/package.zip
Expand Down
46 changes: 22 additions & 24 deletions src/package/packageVersion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export class PackageVersion {
frequency: Duration.seconds(0),
timeout: Duration.seconds(0),
}
): Promise<Partial<PackageVersionCreateRequestResult>> {
): Promise<PackageVersionCreateRequestResult> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function called at the bottom (byId) claims its return type is PackageVersionCreateRequestResult.

Other consumers of byId also return a PackageVersionCreateRequestResult so I used that.

the very loose Partial would have forced lots of undefined checks.

const pvc = new PackageVersionCreate({ ...options });
const createResult = await pvc.createPackageVersion();

Expand Down Expand Up @@ -411,8 +411,9 @@ export class PackageVersion {
}

private static getPackage2VersionFields(connection: Connection): string[] {
const apiVersion = connection.getApiVersion();
return Package2VersionFields.filter((field) => (apiVersion > '60.0' ? true : field !== 'ValidatedAsync'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the conditional didn't need to be checked for every field

return parseInt(connection.getApiVersion(), 10) > 60
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comparing strings via > works fine until we get to 100, after which 100.0 < 99.0 so use numbers when comparing numbers

? Package2VersionFields
: Package2VersionFields.filter((field) => field !== 'ValidatedAsync');
}

/**
Expand Down Expand Up @@ -460,7 +461,7 @@ export class PackageVersion {
if (!this.packageType) {
this.packageType = (
await this.connection.singleRecordQuery<Package2>(
`select ContainerOptions from Package2 where Id = '${await this.getPackageId()}' limit 1`,
`select ContainerOptions from Package2 where Id = '${(await this.getPackageId()) ?? ''}' limit 1`,
{ tooling: true }
)
).ContainerOptions;
Expand All @@ -476,34 +477,29 @@ export class PackageVersion {
* @returns Package2Version
*/
public async getData(force = false): Promise<Package2Version> | never {
let is05i = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope

if (!this.data || force) {
// validate ID
if (this.id.startsWith('04t')) {
validateId(BY_LABEL.SUBSCRIBER_PACKAGE_VERSION_ID, this.id);
is05i = false;
} else if (this.id.startsWith('05i')) {
validateId(BY_LABEL.PACKAGE_VERSION_ID, this.id);
is05i = true;
} else {
throw messages.createError('errorInvalidPackageVersionId', [this.options.idOrAlias]);
}
let queryConfig: { id: string; clause: string; label1: string; label2: string };
if (is05i) {
queryConfig = {
id: this.id,
clause: `Id = '${this.id}'`,
label1: BY_LABEL.PACKAGE_VERSION_ID.label,
label2: BY_LABEL.SUBSCRIBER_PACKAGE_VERSION_ID.label,
};
} else {
queryConfig = {
id: this.id,
clause: `SubscriberPackageVersionId = '${this.id}'`,
label1: BY_LABEL.SUBSCRIBER_PACKAGE_VERSION_ID.label,
label2: BY_LABEL.PACKAGE_VERSION_ID.label,
};
}
const queryConfig: { id: string; clause: string; label1: string; label2: string } = this.id.startsWith('05i')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const > let

? {
id: this.id,
clause: `Id = '${this.id}'`,
label1: BY_LABEL.PACKAGE_VERSION_ID.label,
label2: BY_LABEL.SUBSCRIBER_PACKAGE_VERSION_ID.label,
}
: {
id: this.id,
clause: `SubscriberPackageVersionId = '${this.id}'`,
label1: BY_LABEL.SUBSCRIBER_PACKAGE_VERSION_ID.label,
label2: BY_LABEL.PACKAGE_VERSION_ID.label,
};

const allFields = PackageVersion.getPackage2VersionFields(this.connection).toString();
const query = `SELECT ${allFields} FROM Package2Version WHERE ${queryConfig.clause} LIMIT 1`;
try {
Expand Down Expand Up @@ -633,7 +629,9 @@ export class PackageVersion {
PatchVersion: string;
BuildNumber: string;
}>(
`SELECT Branch, MajorVersion, MinorVersion, PatchVersion, BuildNumber FROM Package2Version WHERE SubscriberPackageVersionId='${results.SubscriberPackageVersionId}'`
`SELECT Branch, MajorVersion, MinorVersion, PatchVersion, BuildNumber FROM Package2Version WHERE SubscriberPackageVersionId='${
results.SubscriberPackageVersionId ?? ''
}'`
)
).records[0];

Expand Down
12 changes: 8 additions & 4 deletions src/package/packageVersionCreate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class PackageVersionCreate {
this.metadataResolver = new MetadataResolver();
}

public createPackageVersion(): Promise<Partial<PackageVersionCreateRequestResult>> {
public createPackageVersion(): Promise<PackageVersionCreateRequestResult> {
try {
return this.packageVersionCreate();
} catch (err) {
Expand Down Expand Up @@ -163,6 +163,7 @@ export class PackageVersionCreate {
return pkgVerQueryResult.records[0].SubscriberPackageVersionId;
}

/** side effect: removes properties from the passed in dependency! */
private async resolveSubscriberPackageVersionId(dependency: PackageDescriptorJson): Promise<PackageDescriptorJson> {
await this.validateDependencyValues(dependency);
if (dependency.subscriberPackageVersionId) {
Expand Down Expand Up @@ -581,14 +582,15 @@ export class PackageVersionCreate {
await settingsGenerator.createDeploy();
await settingsGenerator.createDeployPackageContents(this.apiVersionFromPackageXml);
await zipDir(
`${settingsGenerator.getDestinationPath()}${path.sep}${settingsGenerator.getShapeDirName()}`,
`${settingsGenerator.getDestinationPath() ?? ''}${path.sep}${settingsGenerator.getShapeDirName()}`,
settingsZipFile
);
}
// Zip the Version Info and package.zip files into another zip
await zipDir(packageVersBlobDirectory, packageVersBlobZipFile);
}

/** side effect: modifies the passed in parameter! */
private resolveBuildUserPermissions(packageDescriptorJson: PackageDescriptorJson): void {
// Process permissionSet and permissionSetLicenses that should be enabled when running Apex tests
// This only applies if code coverage is enabled
Expand Down Expand Up @@ -633,7 +635,7 @@ export class PackageVersionCreate {
}

// eslint-disable-next-line complexity
private async packageVersionCreate(): Promise<Partial<PackageVersionCreateRequestResult>> {
private async packageVersionCreate(): Promise<PackageVersionCreateRequestResult> {
// For the first rollout of validating sfdx-project.json data against schema, make it optional and defaulted
// to false. Validation only occurs if the optional validateschema option has been specified.
if (this.options.validateschema) {
Expand Down Expand Up @@ -868,7 +870,9 @@ export class PackageVersionCreate {

const query =
'SELECT Id, IsReleased FROM Package2Version ' +
`WHERE Package2Id = '${packageId}' AND MajorVersion = ${versionNumberSplit[0]} AND MinorVersion = ${versionNumberSplit[1]} AND PatchVersion = ${versionNumberSplit[2]}`;
`WHERE Package2Id = '${packageId ?? ''}' AND MajorVersion = ${versionNumberSplit[0]} AND MinorVersion = ${
versionNumberSplit[1]
} AND PatchVersion = ${versionNumberSplit[2]}`;

let queriedAncestorId: string;
const ancestorVersionResult = await this.connection.tooling.query<PackagingSObjects.Package2Version>(query);
Expand Down
6 changes: 3 additions & 3 deletions src/utils/packageUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { pipeline as cbPipeline } from 'node:stream';
import util, { promisify } from 'node:util';
import { randomBytes } from 'node:crypto';
import { Connection, Logger, Messages, ScratchOrgInfo, SfdcUrl, SfError, SfProject } from '@salesforce/core';
import { isNumber, isString, Many, Nullable, Optional } from '@salesforce/ts-types';
import { isNumber, isString, Many, Optional } from '@salesforce/ts-types';
import type { SaveError } from '@jsforce/jsforce-node';
import { Duration, ensureArray } from '@salesforce/kit';
import globby from 'globby';
Expand Down Expand Up @@ -202,8 +202,8 @@ export async function getPackageVersionId(versionId: string, connection: Connect
});
}

export function escapeInstallationKey(key?: string): Nullable<string> {
return key ? key.replace(/\\/g, '\\\\').replace(/'/g, "\\'") : null;
export function escapeInstallationKey(key: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

every consumer was passing a string

return key.replace(/\\/g, '\\\\').replace(/'/g, "\\'");
}

/**
Expand Down
1 change: 1 addition & 0 deletions test/package/packageVersion.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ describe('Package Version', () => {

describe('create', () => {
it('should include the package version create request ID', async () => {
// @ts-expect-error partial mock
$$.SANDBOX.stub(PackageVersionCreate.prototype, 'createPackageVersion').resolves({
Id: versionCreateRequestId,
});
Expand Down
Loading
Loading