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.

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

feat(envs API) - Support custom dep detectors for envs #7199

Merged
merged 20 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
1 change: 1 addition & 0 deletions fixtures/precinct/bar.foo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
keywords: foo foo foo
1 change: 1 addition & 0 deletions fixtures/precinct/foo.foo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
There is no keywords matched.
15 changes: 14 additions & 1 deletion scopes/dependencies/dependency-resolver/dependencies.service.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import React from 'react';
import { Text, Newline } from 'ink';
import { EnvService, EnvDefinition } from '@teambit/envs';
import { EnvService, EnvDefinition, Env, EnvContext, ServiceTransformationMap } from '@teambit/envs';
import highlight from 'cli-highlight';
import { DependencyDetector } from './dependency-detector';

export type DependenciesDescriptor = {
id: string;
displayName: string;
config?: string;
};

type DependenciesTransformationMap = ServiceTransformationMap & {
getDepDetectors: () => DependencyDetector[] | null;
};

export class DependenciesService implements EnvService<{}, DependenciesDescriptor> {
name = 'Dependencies';

Expand Down Expand Up @@ -36,4 +41,12 @@ export class DependenciesService implements EnvService<{}, DependenciesDescripto
displayName: this.name,
};
}

transform(env: Env, context: EnvContext): DependenciesTransformationMap | undefined {
// Old env
if (!env?.detectors) return undefined;
return {
getDepDetectors: () => env.detectors()(context),
};
}
}
6 changes: 6 additions & 0 deletions scopes/dependencies/dependency-resolver/dependency-env.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { EnvHandler } from '@teambit/envs';
import { DependencyDetector } from './dependency-detector';

export interface DependencyEnv {
detectors?(): EnvHandler<DependencyDetector[] | null>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,20 @@ export class DependencyResolverMain {
return this;
}

/**
* This function called on component load in order to calculate the custom
* dependency detectors from an env, which is got by extension data list.
* Do not use this function for other purposes.
*/
async calcComponentEnvDepDetectors(extensions: ExtensionDataList) {
const envDef = this.envs.calculateEnvFromExtensions(extensions);
const depEnv = envDef.env as DependenciesEnv;
if (typeof depEnv?.getDepDetectors === 'function') {
return depEnv.getDepDetectors();
}
return null;
}

/**
* This function registered to the onLoadRequireableExtensionSlot of the aspect-loader
* Update the aspect / manifest deps versions in the runtimes (recursively)
Expand Down Expand Up @@ -1504,6 +1518,9 @@ export class DependencyResolverMain {
const workspacePolicy = dependencyResolver.getWorkspacePolicy();
return workspacePolicy.toManifest();
});
DependencyResolver.registerEnvDetectorGetter(async (extensions: ExtensionDataList) => {
return dependencyResolver.calcComponentEnvDepDetectors(extensions);
});
DependencyResolver.registerHarmonyEnvPeersPolicyForEnvItselfGetter(async (id: BitId, files: SourceFile[]) => {
const envPolicy = await dependencyResolver.getEnvPolicyFromEnvLegacyId(id, files);
if (!envPolicy) return undefined;
Expand Down
1 change: 1 addition & 0 deletions scopes/dependencies/dependency-resolver/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,4 @@ export { DependencySource, VariantPolicyEntry } from './policy/variant-policy/va
export { OutdatedPkg } from './get-all-policy-pkgs';
export { extendWithComponentsFromDir } from './extend-with-components-from-dir';
export { isRange } from './manifest/deduping/hoist-dependencies';
export { DependencyEnv } from './dependency-env';
8 changes: 7 additions & 1 deletion scopes/envs/envs/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { BuildTask } from '@teambit/builder';
import type { SchemaExtractor } from '@teambit/schema';
import type { WebpackConfigTransformer } from '@teambit/webpack';
import type { PackageJsonProps } from '@teambit/pkg';
import type { EnvPolicyConfigObject } from '@teambit/dependency-resolver';
import type { DependencyDetector, EnvPolicyConfigObject } from '@teambit/dependency-resolver';
import { ElementsWrapperContext } from '@teambit/elements';
import type { Capsule } from '@teambit/isolator';
import type { Component } from '@teambit/component';
Expand Down Expand Up @@ -90,6 +90,12 @@ export interface DependenciesEnv extends Environment {
* by default bit will merge this list with the peers from the getDependencies function
*/
getAdditionalHostDependencies?: () => string[] | Promise<string[]>;

/**
* Returns a list of dependency detectors
* this list will be used to detect all the dependencies in each file of the component
*/
getDepDetectors?: () => DependencyDetector[] | null;
}

export type GetNpmIgnoreContext = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { ExtensionDataList } from '../../../config';
import PackageJsonFile from '../../../../consumer/component/package-json-file';
import { SourceFile } from '../../sources';
import { DependenciesOverridesData } from '../../../config/component-overrides';
import { DependencyDetector } from '../files-dependency-builder/detector-hook';

export type AllDependencies = {
dependencies: Dependency[];
Expand Down Expand Up @@ -91,6 +92,8 @@ const DepsKeysToAllPackagesDepsKeys = {
peerDependencies: 'peerPackageDependencies',
};

type GetEnvDetectors = (extensions: ExtensionDataList) => Promise<DependencyDetector[] | null>;

export default class DependencyResolver {
component: Component;
consumer: Consumer;
Expand Down Expand Up @@ -119,6 +122,11 @@ export default class DependencyResolver {
this.getWorkspacePolicy = func;
}

static envDetectorsGetter: GetEnvDetectors;
static registerEnvDetectorGetter(getter: GetEnvDetectors) {
this.envDetectorsGetter = getter;
}

static getOnComponentAutoDetectOverrides: OnComponentAutoDetectOverrides;
static registerOnComponentAutoDetectOverridesGetter(func: OnComponentAutoDetectOverrides) {
this.getOnComponentAutoDetectOverrides = func;
Expand Down Expand Up @@ -199,6 +207,7 @@ export default class DependencyResolver {
: this.consumerPath;
const { nonTestsFiles, testsFiles } = this.componentMap.getFilesGroupedByBeingTests();
const allFiles = [...nonTestsFiles, ...testsFiles];
const envDetectors = await this.getEnvDetectors();
// find the dependencies (internal files and packages) through automatic dependency resolution
const dependenciesTree = await getDependencyTree({
componentDir,
Expand All @@ -208,6 +217,7 @@ export default class DependencyResolver {
resolveModulesConfig: this.consumer.config._resolveModules,
visited: cacheResolvedDependencies,
cacheProjectAst,
envDetectors,
});
// we have the files dependencies, these files should be components that are registered in bit.map. Otherwise,
// they are referred as "untracked components" and the user should add them later on in order to tag
Expand All @@ -221,6 +231,10 @@ export default class DependencyResolver {
});
}

async getEnvDetectors(): Promise<DependencyDetector[] | null> {
return DependencyResolver.envDetectorsGetter(this.component.extensions);
}

/**
* Given the tree of file dependencies from the driver, find the components of these files.
* Each dependency file has a path, use bit.map to search for the component name by that path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ export async function getDependencyTree({
resolveModulesConfig,
visited = {},
cacheProjectAst,
envDetectors,
}: DependencyTreeParams): Promise<{ tree: DependenciesTree }> {
const resolveConfigAbsolute = getResolveConfigAbsolute(workspacePath, resolveModulesConfig);
const config = {
Expand All @@ -235,6 +236,7 @@ export async function getDependencyTree({
nonExistent: [],
resolveConfig: resolveConfigAbsolute,
cacheProjectAst,
envDetectors,
};
// This is important because without this, madge won't know to resolve files if we run the
// CMD not from the root dir
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export default function Config(options) {
this.resolveConfig = options.resolveConfig;
// @ts-ignore AUTO-ADDED-AFTER-MIGRATION-PLEASE-FIX!
this.cacheProjectAst = options.cacheProjectAst;
// @ts-ignore AUTO-ADDED-AFTER-MIGRATION-PLEASE-FIX!
this.envDetectors = options.envDetectors;

// @ts-ignore AUTO-ADDED-AFTER-MIGRATION-PLEASE-FIX!
this.filter = options.filter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export default function (options) {
debug(`file ${config.filename} does not exist`);
return {};
}

return traverse(config);
}

Expand Down Expand Up @@ -65,6 +64,7 @@ module.exports._getDependencies = function (config) {
let dependenciesRaw; // from some detectives it comes as an array, from some it is an object
const precinctOptions = config.detectiveConfig;
precinctOptions.includeCore = false;
precinctOptions.envDetectors = config.envDetectors;
// @ts-ignore
delete precinct.ast;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ export interface DependencyDetector {
*/
isSupported(context: FileContext): boolean;

/**
* determine what type of content the detector is for.
* by default, the type is the extension name of the file (without the dot)
* if no type provided.
*/
type?: string;
Copy link
Member

Choose a reason for hiding this comment

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

What possible values are for this?
if it's general string, we should improve the comment above it to explain what it is

Copy link
Member Author

Choose a reason for hiding this comment

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

This type is what precinct() previously receives as the second argument. Also, it's what the previous getType() returns in paperwork(). I added this field into the detector interface to combine the 2 steps directly.

The previous implementation has 2 steps: 1) get the type from the ext, 2) get detective from the type.

Sometimes ext names and types are not 1:1 matched. For example, .ts and .tsx both match the type ts.

For custom env detectors, I think it's also a good practice for detector authors to specify what type of files it belongs to.

And I updated the code comment a little bit. What do you think now? :-)


/**
* detect file dependencies. list of file dependencies of the module.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export default function generateTree(files: string[] = [], config): GenerateTree
nonExistent,
pathMap,
cacheProjectAst: config.cacheProjectAst,
envDetectors: config.envDetectors,
});
Object.assign(depTree, dependencyTreeResult);
} catch (err: any) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect } from 'chai';
import { DependencyDetector } from '../detector-hook';

const assert = require('assert');
const fs = require('fs');
Expand Down Expand Up @@ -200,8 +201,30 @@ describe('node-precinct', () => {
assert.deepEqual(deps, ['./a', './b']);
});

it('supports passing env detectors', () => {
const detector: DependencyDetector = {
detect: (fileContent: string) => {
return fileContent.indexOf('foo') === -1 ? [] : ['foo'];
},
isSupported: ({ ext }) => {
return ext === '.foo';
},
type: 'foo',
};
const result = precinct.paperwork(`${fixturesFullPath}/foo.foo`, {
envDetectors: [detector],
});
assert.deepEqual(result, []);

const result2 = precinct.paperwork(`${fixturesFullPath}/bar.foo`, {
envDetectors: [detector],
});
assert.deepEqual(result2, ['foo']);
});

describe('when given detective configuration', () => {
it('still does not filter out core module by default', () => {
// This test case doesn't fit the current implementation of precinct.
it.skip('still does not filter out core module by default', () => {
const stub = sinon.stub().returns([]);
const revert = precinctNonWired.__set__('precinct', stub);

Expand Down
Loading