Skip to content

Commit

Permalink
refactor(core)!: remove workspace.locator (#5619)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**
<!-- Describe the rationale of your PR. -->
<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->

`workspace.locator` is quite useless and only creates confusion with
`workspace.anchoredLocator`.

At the moment, it's only used in places that need an ident, which
`workspace.anchoredLocator` can happily provide.

In the past, it was also used accidentally instead of
`workspace.anchoredLocator` before I fixed it in
#4937.

**How did you fix it?**
<!-- A detailed description of your implementation. -->

Removed it since it doesn't provide any benefits. Its only actual use is
comparisons in `project.tryWorkspaceByLocator`, but nobody has ever
passed it to `project.tryWorkspaceByLocator` anyways.

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [X] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [X] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [X] I will check that all automated PR checks pass before the PR gets
reviewed.
  • Loading branch information
paul-soporan authored Jul 28, 2023
1 parent d2fa9ce commit bca874d
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 29 deletions.
34 changes: 34 additions & 0 deletions .yarn/versions/e5885c61.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
releases:
"@yarnpkg/cli": major
"@yarnpkg/core": major
"@yarnpkg/doctor": patch
"@yarnpkg/plugin-constraints": patch
"@yarnpkg/plugin-essentials": patch
"@yarnpkg/plugin-nm": patch
"@yarnpkg/plugin-version": patch
"@yarnpkg/plugin-workspace-tools": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-exec"
- "@yarnpkg/plugin-file"
- "@yarnpkg/plugin-git"
- "@yarnpkg/plugin-github"
- "@yarnpkg/plugin-http"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-link"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/builder"
- "@yarnpkg/extensions"
- "@yarnpkg/nm"
- "@yarnpkg/pnpify"
- "@yarnpkg/sdks"
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ The following changes only affect people writing Yarn plugins:

- `forgettableNames` & `forgettableBufferSize` have been removed (the only messages using them have been removed, making the forgettable logs implementation obsolete).

- `workspace.locator` has been removed. You can instead use:
- `workspace.anchoredLocator` to get the locator that's used throughout the dependency tree.
- `workspace.manifest.version` to get the workspace version.

### Installs

- Yarn now caches npm version metadata, leading to faster resolution steps and decreased network data usage.
Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-constraints/sources/Constraints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export class Constraints implements constraintUtils.Engine {
const relativeCwd = workspace.relativeCwd;

database += `workspace(${escape(relativeCwd)}).\n`;
database += `workspace_ident(${escape(relativeCwd)}, ${escape(structUtils.stringifyIdent(workspace.locator))}).\n`;
database += `workspace_ident(${escape(relativeCwd)}, ${escape(structUtils.stringifyIdent(workspace.anchoredLocator))}).\n`;
database += `workspace_version(${escape(relativeCwd)}, ${escape(workspace.manifest.version)}).\n`;

for (const dependencyType of DEPENDENCY_TYPES) {
Expand Down Expand Up @@ -299,7 +299,7 @@ export class Constraints implements constraintUtils.Engine {

return miscUtils.sortMap(enforcedDependencies, [
({dependencyRange}) => dependencyRange !== null ? `0` : `1`,
({workspace}) => structUtils.stringifyIdent(workspace.locator),
({workspace}) => structUtils.stringifyIdent(workspace.anchoredLocator),
({dependencyIdent}) => structUtils.stringifyIdent(dependencyIdent),
]);
}
Expand All @@ -321,7 +321,7 @@ export class Constraints implements constraintUtils.Engine {
}

return miscUtils.sortMap(enforcedFields, [
({workspace}) => structUtils.stringifyIdent(workspace.locator),
({workspace}) => structUtils.stringifyIdent(workspace.anchoredLocator),
({fieldPath}) => fieldPath,
]);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-constraints/sources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const plugin: Plugin<Hooks> = {
if (project.configuration.isCI) {
for (const [workspace, workspaceErrors] of remainingErrors) {
for (const error of workspaceErrors) {
reportError(MessageName.CONSTRAINTS_CHECK_FAILED, `${formatUtils.pretty(project.configuration, workspace.locator, formatUtils.Type.IDENT)}: ${error.text}`);
reportError(MessageName.CONSTRAINTS_CHECK_FAILED, `${formatUtils.pretty(project.configuration, workspace.anchoredLocator, formatUtils.Type.IDENT)}: ${error.text}`);
}
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-essentials/sources/commands/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export default class LinkCommand extends BaseCommand {
}

for (const workspace of linkedWorkspaces) {
const fullName = structUtils.stringifyIdent(workspace.locator);
const fullName = structUtils.stringifyIdent(workspace.anchoredLocator);
const target = this.relative
? ppath.relative(project.cwd, workspace.cwd)
: workspace.cwd;
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-essentials/sources/commands/unlink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export default class UnlinkCommand extends BaseCommand {
if (this.all) {
for (const workspace of project2.workspaces)
if (workspace.manifest.name)
workspacesToUnlink.add(structUtils.stringifyIdent(workspace.locator));
workspacesToUnlink.add(structUtils.stringifyIdent(workspace.anchoredLocator));

if (workspacesToUnlink.size === 0) {
throw new UsageError(`No workspace found to be unlinked in the target project`);
Expand All @@ -77,7 +77,7 @@ export default class UnlinkCommand extends BaseCommand {
if (!workspace2.manifest.name)
throw new UsageError(`The target workspace doesn't have a name and thus cannot be unlinked`);

workspacesToUnlink.add(structUtils.stringifyIdent(workspace2.locator));
workspacesToUnlink.add(structUtils.stringifyIdent(workspace2.anchoredLocator));
}
} else {
const fullNames = [...topLevelWorkspace.manifest.resolutions.map(({pattern}) => pattern.descriptor.fullName)];
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-essentials/sources/commands/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export default class WorkspaceCommand extends BaseCommand {
const candidates = project.workspaces;
const candidatesByName = new Map(
candidates.map(
workspace => [structUtils.stringifyIdent(workspace.locator), workspace],
workspace => [structUtils.stringifyIdent(workspace.anchoredLocator), workspace],
),
);

Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-nm/sources/NodeModulesLinker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ class NodeModulesInstaller implements Installer {
getDependencyTreeRoots: () => {
return this.opts.project.workspaces.map(workspace => {
const anchoredLocator = workspace.anchoredLocator;
return {name: structUtils.stringifyIdent(workspace.locator), reference: anchoredLocator.reference};
return {name: structUtils.stringifyIdent(anchoredLocator), reference: anchoredLocator.reference};
});
},
getPackageInformation: pnpLocator => {
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-version/sources/versionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export async function openVersionFile(project: Project, {allowEmpty = false}: {a
if (workspace.manifest.version === null)
continue;

const identStr = structUtils.stringifyIdent(workspace.locator);
const identStr = structUtils.stringifyIdent(workspace.anchoredLocator);

const decision = releaseStore.get(workspace);
if (decision === Decision.DECLINE) {
Expand Down
8 changes: 4 additions & 4 deletions packages/plugin-workspace-tools/sources/commands/foreach.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export default class WorkspacesForeachCommand extends BaseCommand {
? Array.from(await gitUtils.fetchChangedWorkspaces({ref: this.since, project}))
: [rootWorkspace, ...(this.from.length > 0 ? rootWorkspace.getRecursiveWorkspaceChildren() : [])];

const fromPredicate = (workspace: Workspace) => micromatch.isMatch(structUtils.stringifyIdent(workspace.locator), this.from) || micromatch.isMatch(workspace.relativeCwd, this.from);
const fromPredicate = (workspace: Workspace) => micromatch.isMatch(structUtils.stringifyIdent(workspace.anchoredLocator), this.from) || micromatch.isMatch(workspace.relativeCwd, this.from);
const fromCandidates: Array<Workspace> = this.from.length > 0
? rootCandidates.filter(fromPredicate)
: rootCandidates;
Expand Down Expand Up @@ -181,10 +181,10 @@ export default class WorkspacesForeachCommand extends BaseCommand {
if (scriptName === configuration.env.npm_lifecycle_event && workspace.cwd === cwdWorkspace!.cwd)
continue;

if (this.include.length > 0 && !micromatch.isMatch(structUtils.stringifyIdent(workspace.locator), this.include) && !micromatch.isMatch(workspace.relativeCwd, this.include))
if (this.include.length > 0 && !micromatch.isMatch(structUtils.stringifyIdent(workspace.anchoredLocator), this.include) && !micromatch.isMatch(workspace.relativeCwd, this.include))
continue;

if (this.exclude.length > 0 && (micromatch.isMatch(structUtils.stringifyIdent(workspace.locator), this.exclude) || micromatch.isMatch(workspace.relativeCwd, this.exclude)))
if (this.exclude.length > 0 && (micromatch.isMatch(structUtils.stringifyIdent(workspace.anchoredLocator), this.exclude) || micromatch.isMatch(workspace.relativeCwd, this.exclude)))
continue;

if (this.publicOnly && workspace.manifest.private === true)
Expand Down Expand Up @@ -400,7 +400,7 @@ function getPrefix(workspace: Workspace, {configuration, commandIndex, verbose}:
if (!verbose)
return null;

const name = structUtils.stringifyIdent(workspace.locator);
const name = structUtils.stringifyIdent(workspace.anchoredLocator);

const prefix = `[${name}]:`;

Expand Down
8 changes: 4 additions & 4 deletions packages/yarnpkg-core/sources/Project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,14 +445,14 @@ export class Project {
}

private addWorkspace(workspace: Workspace) {
const dup = this.workspacesByIdent.get(workspace.locator.identHash);
const dup = this.workspacesByIdent.get(workspace.anchoredLocator.identHash);
if (typeof dup !== `undefined`)
throw new Error(`Duplicate workspace name ${structUtils.prettyIdent(this.configuration, workspace.locator)}: ${npath.fromPortablePath(workspace.cwd)} conflicts with ${npath.fromPortablePath(dup.cwd)}`);
throw new Error(`Duplicate workspace name ${structUtils.prettyIdent(this.configuration, workspace.anchoredLocator)}: ${npath.fromPortablePath(workspace.cwd)} conflicts with ${npath.fromPortablePath(dup.cwd)}`);

this.workspaces.push(workspace);

this.workspacesByCwd.set(workspace.cwd, workspace);
this.workspacesByIdent.set(workspace.locator.identHash, workspace);
this.workspacesByIdent.set(workspace.anchoredLocator.identHash, workspace);
}

get topLevelWorkspace() {
Expand Down Expand Up @@ -565,7 +565,7 @@ export class Project {
if (structUtils.isVirtualLocator(locator))
locator = structUtils.devirtualizeLocator(locator);

if (workspace.locator.locatorHash !== locator.locatorHash && workspace.anchoredLocator.locatorHash !== locator.locatorHash)
if (workspace.anchoredLocator.locatorHash !== locator.locatorHash)
return null;

return workspace;
Expand Down
11 changes: 2 additions & 9 deletions packages/yarnpkg-core/sources/Workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ export class Workspace {
// @ts-expect-error: This variable is set during the setup process
public readonly anchoredLocator: Locator;

// @ts-expect-error: This variable is set during the setup process
public readonly locator: Locator;

public readonly workspacesCwds: Set<PortablePath> = new Set();

// @ts-expect-error: This variable is set during the setup process
Expand All @@ -44,16 +41,12 @@ export class Workspace {
this.relativeCwd = ppath.relative(this.project.cwd, this.cwd) || PortablePath.dot;

const ident = this.manifest.name ? this.manifest.name : structUtils.makeIdent(null, `${this.computeCandidateName()}-${hashUtils.makeHash<string>(this.relativeCwd).substring(0, 6)}`);
const reference = this.manifest.version ? this.manifest.version : `0.0.0`;

// @ts-expect-error: It's ok to initialize it now, even if it's readonly (setup is called right after construction)
this.locator = structUtils.makeLocator(ident, reference);

// @ts-expect-error: It's ok to initialize it now, even if it's readonly (setup is called right after construction)
this.anchoredDescriptor = structUtils.makeDescriptor(this.locator, `${WorkspaceResolver.protocol}${this.relativeCwd}`);
this.anchoredDescriptor = structUtils.makeDescriptor(ident, `${WorkspaceResolver.protocol}${this.relativeCwd}`);

// @ts-expect-error: It's ok to initialize it now, even if it's readonly (setup is called right after construction)
this.anchoredLocator = structUtils.makeLocator(this.locator, `${WorkspaceResolver.protocol}${this.relativeCwd}`);
this.anchoredLocator = structUtils.makeLocator(ident, `${WorkspaceResolver.protocol}${this.relativeCwd}`);

const patterns = this.manifest.workspaceDefinitions.map(({pattern}) => pattern);

Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-core/sources/structUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ export function sortDescriptors(descriptors: Iterable<Descriptor>) {
* @param workspace The workspace to pretty print
*/
export function prettyWorkspace(configuration: Configuration, workspace: Workspace) {
return prettyIdent(configuration, workspace.locator);
return prettyIdent(configuration, workspace.anchoredLocator);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-doctor/sources/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ function extractIdents(name: string) {
}

function isValidDependency(ident: Ident, {workspace}: {workspace: Workspace}) {
if (ident.identHash === workspace.locator.identHash)
if (ident.identHash === workspace.anchoredLocator.identHash)
return true;

if (workspace.manifest.hasDependency(ident))
Expand Down

0 comments on commit bca874d

Please sign in to comment.