Skip to content

Commit

Permalink
Fix circular dependencies on @parcel/types (#9610)
Browse files Browse the repository at this point in the history
* Fix all circular dependencies

* Fix circular dependencies between packages and `@parcel/types`
* Use generic WorkerFarm type parameter to avoid having to move/duplicate the
  class

* Don't do deep imports to types

* Fix accidental changes to package.json

* Revert turbo related changes to package.json

* Fix typescript errors

* Fix more typescript errors

* Move packages into a types-internal pkg

This allows for there to be no API changes from the circular dependencies fix.

There is now a `@parcel/types` package with the same API as before and a new
`@parcel/types-internal` package, which is consumed by `@parcel/workers` while
avoiding it.

* Fix typescript issues

* Fix eslint errors

* Fix typescript errors

* Add back fs dependency on parcel/types

* Add missing exports

* Fix dependency cycle

* Fix package-manager breaking changes

* Fix import duplication in ts declaration file

---------

Co-authored-by: Pedro Yamada <[email protected]>
  • Loading branch information
yamadapc and pyamada-atlassian authored Apr 10, 2024
1 parent 2a75be0 commit 119f186
Show file tree
Hide file tree
Showing 51 changed files with 310 additions and 143 deletions.
1 change: 1 addition & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const IGNORED_PACKAGES = [
'!packages/core/workers/test/integration/**',
'!packages/core/test-utils/**',
'!packages/core/types/**',
'!packages/core/types-internal/**',

// These packages are bundled.
'!packages/core/codeframe/**',
Expand Down
4 changes: 2 additions & 2 deletions packages/core/cache/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import type {Cache} from './lib/types';

export type {Cache} from './lib/types';
export const FSCache: {
new (cacheDir: FilePath): Cache
new (cacheDir: FilePath): Cache;
};

export const LMDBCache: {
new (cacheDir: FilePath): Cache
new (cacheDir: FilePath): Cache;
};
27 changes: 2 additions & 25 deletions packages/core/cache/src/types.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,4 @@
// @flow
import type {Readable} from 'stream';
import type {Cache} from '@parcel/types';

export interface Cache {
ensure(): Promise<void>;
has(key: string): Promise<boolean>;
get<T>(key: string): Promise<?T>;
set(key: string, value: mixed): Promise<void>;
getStream(key: string): Readable;
setStream(key: string, stream: Readable): Promise<void>;
getBlob(key: string): Promise<Buffer>;
setBlob(key: string, contents: Buffer | string): Promise<void>;
hasLargeBlob(key: string): Promise<boolean>;
getLargeBlob(key: string): Promise<Buffer>;
setLargeBlob(
key: string,
contents: Buffer | string,
options?: {|signal?: AbortSignal|},
): Promise<void>;
getBuffer(key: string): Promise<?Buffer>;
/**
* In a multi-threaded environment, where there are potentially multiple Cache
* instances writing to the cache, ensure that this instance has the latest view
* of the changes that may have been written to the cache in other threads.
*/
refresh(): void;
}
export type {Cache};
13 changes: 10 additions & 3 deletions packages/core/core/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import type {InitialParcelOptions, BuildEvent, BuildSuccessEvent, AsyncSubscription} from '@parcel/types';
import type {
InitialParcelOptions,
BuildEvent,
BuildSuccessEvent,
AsyncSubscription,
} from '@parcel/types';
import type {FarmOptions} from '@parcel/workers';
import type WorkerFarm from '@parcel/workers';

Expand All @@ -7,9 +12,11 @@ export class Parcel {
run(): Promise<BuildSuccessEvent>;
watch(
cb?: (err: Error | null | undefined, buildEvent?: BuildEvent) => unknown,
): Promise<AsyncSubscription>
): Promise<AsyncSubscription>;
}

export declare function createWorkerFarm(options?: Partial<FarmOptions>): WorkerFarm;
export declare function createWorkerFarm(
options?: Partial<FarmOptions>,
): WorkerFarm;

export default Parcel;
2 changes: 1 addition & 1 deletion packages/core/core/src/Parcel.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default class Parcel {
#farm /*: WorkerFarm*/;
#initialized /*: boolean*/ = false;
#disposable /*: Disposable */;
#initialOptions /*: InitialParcelOptions*/;
#initialOptions /*: InitialParcelOptions */;
#reporterRunner /*: ReporterRunner*/;
#resolvedOptions /*: ?ParcelOptions*/ = null;
#optionsRef /*: SharedReference */;
Expand Down
11 changes: 9 additions & 2 deletions packages/core/fs/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import type {FileSystem} from './lib/types';
import type {FileSystem} from '@parcel/types-internal';
import type WorkerFarm from '@parcel/workers';

export * from './lib/types';
export type {
FileSystem,
FileOptions,
ReaddirOptions,
Stats,
Encoding,
Dirent,
} from '@parcel/types-internal';

export const NodeFS: {
new (): FileSystem;
Expand Down
6 changes: 3 additions & 3 deletions packages/core/fs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"includeNodeModules": {
"@parcel/core": false,
"@parcel/rust": false,
"@parcel/types": false,
"@parcel/types-internal": false,
"@parcel/utils": false,
"@parcel/watcher": false,
"@parcel/workers": false
Expand All @@ -36,7 +36,7 @@
"includeNodeModules": {
"@parcel/core": false,
"@parcel/rust": false,
"@parcel/types": false,
"@parcel/types-internal": false,
"@parcel/utils": false,
"@parcel/watcher": false,
"@parcel/workers": false
Expand All @@ -49,7 +49,7 @@
},
"dependencies": {
"@parcel/rust": "2.12.0",
"@parcel/types": "2.12.0",
"@parcel/types-internal": "2.12.0",
"@parcel/utils": "2.12.0",
"@parcel/watcher": "^2.0.7",
"@parcel/workers": "2.12.0"
Expand Down
9 changes: 7 additions & 2 deletions packages/core/fs/src/MemoryFS.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
// @flow

import type {FileSystem, FileOptions, ReaddirOptions, Encoding} from './types';
import type {FilePath} from '@parcel/types';
import type {
FilePath,
FileSystem,
FileOptions,
ReaddirOptions,
Encoding,
} from '@parcel/types-internal';
import type {
Event,
Options as WatcherOptions,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/fs/src/NodeFS.browser.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @flow
import type {FileSystem} from './types';
import type {FileSystem} from '@parcel/types-internal';

// $FlowFixMe[prop-missing] handled by the throwing constructor
export class NodeFS implements FileSystem {
Expand Down
8 changes: 6 additions & 2 deletions packages/core/fs/src/NodeFS.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
// @flow
import type {ReadStream, Stats} from 'fs';
import type {Writable} from 'stream';
import type {FileOptions, FileSystem, Encoding} from './types';
import type {FilePath} from '@parcel/types';
import type {
FilePath,
Encoding,
FileOptions,
FileSystem,
} from '@parcel/types-internal';
import type {
Event,
Options as WatcherOptions,
Expand Down
10 changes: 5 additions & 5 deletions packages/core/fs/src/OverlayFS.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

import type {Readable, Writable} from 'stream';
import type {
FilePath,
Encoding,
FileOptions,
FileSystem,
ReaddirOptions,
Stats,
} from './types';
import type {FilePath} from '@parcel/types';
FileStats,
} from '@parcel/types-internal';
import type {
Event,
Options as WatcherOptions,
Expand Down Expand Up @@ -168,7 +168,7 @@ export class OverlayFS implements FileSystem {
}

// eslint-disable-next-line require-await
async stat(filePath: FilePath): Promise<Stats> {
async stat(filePath: FilePath): Promise<FileStats> {
return this.statSync(filePath);
}

Expand Down Expand Up @@ -289,7 +289,7 @@ export class OverlayFS implements FileSystem {
}
}

statSync(filePath: FilePath): Stats {
statSync(filePath: FilePath): FileStats {
filePath = this._normalizePath(filePath);
try {
return this.writable.statSync(filePath);
Expand Down
3 changes: 1 addition & 2 deletions packages/core/fs/src/find.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// @flow
import type {FilePath} from '@parcel/types';
import type {FileSystem} from './types';
import type {FilePath, FileSystem} from '@parcel/types-internal';
import path from 'path';

export function findNodeModule(
Expand Down
6 changes: 3 additions & 3 deletions packages/core/fs/src/index.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
// @flow strict-local
import type {FileSystem} from './types';
import type {FilePath} from '@parcel/types';
import type {FilePath, FileSystem, FileOptions} from '@parcel/types-internal';
import type {Readable, Writable} from 'stream';

import path from 'path';
import stream from 'stream';
import {promisify} from 'util';

export type * from './types';
export * from './NodeFS';
export * from './MemoryFS';
export * from './OverlayFS';

export type {FileSystem, FileOptions};

const pipeline: (Readable, Writable) => Promise<void> = promisify(
stream.pipeline,
);
Expand Down
26 changes: 22 additions & 4 deletions packages/core/package-manager/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
import type {FilePath} from '@parcel/types';
import type {
FilePath,
PackageInstaller,
PackageManager,
PackageManagerResolveResult,
} from '@parcel/types';
import type {FileSystem} from '@parcel/fs';
import type {PackageInstaller, PackageManager} from './lib/types';

export * from './lib/types';
export type {PackageManagerResolveResult};
export type {PackageManagerResolveResult as ResolveResult};

export type {
PackageManager,
InstallOptions,
InstallerOptions,
PackageInstaller,
Invalidations,
ModuleRequest,
} from '@parcel/types';

export const Npm: {
new (): PackageInstaller;
Expand All @@ -18,5 +32,9 @@ export const MockPackageInstaller: {
new (): PackageInstaller;
};
export const NodePackageManager: {
new (fs: FileSystem, projectRoot: FilePath, installer?: PackageInstaller): PackageManager;
new (
fs: FileSystem,
projectRoot: FilePath,
installer?: PackageInstaller,
): PackageManager;
};
2 changes: 1 addition & 1 deletion packages/core/package-manager/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"node": ">= 12.0.0"
},
"scripts": {
"build-ts": "mkdir -p lib && flow-to-ts src/types.js > lib/types.d.ts",
"build-ts": "mkdir -p lib && flow-to-ts src/index.js > lib/index.d.ts",
"check-ts": "tsc --noEmit index.d.ts",
"test": "mocha test"
},
Expand Down
6 changes: 5 additions & 1 deletion packages/core/package-manager/src/MockPackageInstaller.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// @flow

import type {ModuleRequest, PackageInstaller, InstallerOptions} from './types';
import type {
ModuleRequest,
PackageInstaller,
InstallerOptions,
} from '@parcel/types';
import type {FileSystem} from '@parcel/fs';
import type {FilePath} from '@parcel/types';

Expand Down
15 changes: 9 additions & 6 deletions packages/core/package-manager/src/NodePackageManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import type {
PackageInstaller,
InstallOptions,
Invalidations,
} from './types';
import type {ResolveResult} from './types';
PackageManagerResolveResult,
} from '@parcel/types';

import {registerSerializableClass} from '@parcel/core';
import ThrowableDiagnostic, {
Expand Down Expand Up @@ -47,7 +47,7 @@ const NODE_MODULES = `${path.sep}node_modules${path.sep}`;

// There can be more than one instance of NodePackageManager, but node has only a single module cache.
// Therefore, the resolution cache and the map of parent to child modules should also be global.
const cache = new Map<DependencySpecifier, ResolveResult>();
const cache = new Map<DependencySpecifier, PackageManagerResolveResult>();
const children = new Map<FilePath, Set<DependencySpecifier>>();
const invalidationsCache = new Map<string, Invalidations>();

Expand Down Expand Up @@ -259,7 +259,7 @@ export class NodePackageManager implements PackageManager {
shouldAutoInstall?: boolean,
saveDev?: boolean,
|},
): Promise<ResolveResult> {
): Promise<PackageManagerResolveResult> {
let basedir = path.dirname(from);
let key = basedir + ':' + id;
let resolved = cache.get(key);
Expand Down Expand Up @@ -413,7 +413,10 @@ export class NodePackageManager implements PackageManager {
return resolved;
}
resolveSync(name: DependencySpecifier, from: FilePath): ResolveResult {
resolveSync(
name: DependencySpecifier,
from: FilePath,
): PackageManagerResolveResult {
let basedir = path.dirname(from);
let key = basedir + ':' + name;
let resolved = cache.get(key);
Expand Down Expand Up @@ -568,7 +571,7 @@ export class NodePackageManager implements PackageManager {
this.resolver = this._createResolver();
}
resolveInternal(name: string, from: string): ResolveResult {
resolveInternal(name: string, from: string): PackageManagerResolveResult {
if (this.resolver == null) {
this.resolver = this._createResolver();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package-manager/src/Npm.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow strict-local

import type {PackageInstaller, InstallerOptions} from './types';
import type {PackageInstaller, InstallerOptions} from '@parcel/types';

import path from 'path';
import spawn from 'cross-spawn';
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package-manager/src/Pnpm.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow strict-local

import type {PackageInstaller, InstallerOptions} from './types';
import type {PackageInstaller, InstallerOptions} from '@parcel/types';

import path from 'path';
import fs from 'fs';
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package-manager/src/Yarn.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow strict-local

import type {PackageInstaller, InstallerOptions} from './types';
import type {PackageInstaller, InstallerOptions} from '@parcel/types';

import commandExists from 'command-exists';
import spawn from 'cross-spawn';
Expand Down
13 changes: 12 additions & 1 deletion packages/core/package-manager/src/index.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
// @flow
export type * from './types';

import type {PackageManagerResolveResult} from '@parcel/types';

export type {
PackageManager,
Invalidations,
PackageInstaller,
ModuleRequest,
} from '@parcel/types';
export * from './Npm';
export * from './Pnpm';
export * from './Yarn';
export * from './MockPackageInstaller';
export * from './NodePackageManager';
export {_addToInstallQueue} from './installPackage';

export type {PackageManagerResolveResult};
export type {PackageManagerResolveResult as ResolveResult};
2 changes: 1 addition & 1 deletion packages/core/package-manager/src/installPackage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
PackageManager,
PackageInstaller,
InstallOptions,
} from './types';
} from '@parcel/types';
import type {FileSystem} from '@parcel/fs';

import invariant from 'assert';
Expand Down
3 changes: 1 addition & 2 deletions packages/core/package-manager/src/utils.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @flow strict-local

import type {ModuleRequest} from './types';
import type {FilePath} from '@parcel/types';
import type {FilePath, ModuleRequest} from '@parcel/types';
import type {FileSystem} from '@parcel/fs';

import invariant from 'assert';
Expand Down
Loading

0 comments on commit 119f186

Please sign in to comment.