Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover committed Mar 28, 2019
1 parent 6e12380 commit 0ce12c5
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 31 deletions.
1 change: 0 additions & 1 deletion src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,6 @@ export default function (program) {
repl: !!opts.repl,
basePath: !!opts.basePath,
optimize: !!opts.optimize,
pluginPath: opts.pluginPath,
},
features: {
isClusterModeSupported: CAN_CLUSTER,
Expand Down
10 changes: 1 addition & 9 deletions src/core/server/config/__mocks__/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,9 @@

import { EnvOptions } from '../env';

type Optional<T> = T | undefined;

// Prettier changes this syntax to be invalid.
// tslint:disable prettier
type DeepPartial<T> = {
[P in keyof T]?:
T[P] extends Optional<Array<infer R>> ? Optional<R[]> :
T[P] extends Array<infer U> ? Array<DeepPartial<U>> : DeepPartial<T[P]>
[P in keyof T]?: T[P] extends Array<infer R> ? Array<DeepPartial<R>> : DeepPartial<T[P]>
};
// tslint:enable prettier

export function getEnvOptions(options: DeepPartial<EnvOptions> = {}): EnvOptions {
return {
Expand All @@ -44,7 +37,6 @@ export function getEnvOptions(options: DeepPartial<EnvOptions> = {}): EnvOptions
repl: false,
basePath: false,
optimize: false,
pluginPath: undefined,
...(options.cliArgs || {}),
},
isDevClusterMaster:
Expand Down
6 changes: 0 additions & 6 deletions src/core/server/config/__snapshots__/env.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ Env {
"envName": "development",
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": false,
Expand Down Expand Up @@ -51,7 +50,6 @@ Env {
"envName": "production",
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": false,
Expand Down Expand Up @@ -92,7 +90,6 @@ Env {
"dev": true,
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": false,
Expand Down Expand Up @@ -133,7 +130,6 @@ Env {
"dev": false,
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": false,
Expand Down Expand Up @@ -174,7 +170,6 @@ Env {
"dev": false,
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": false,
Expand Down Expand Up @@ -215,7 +210,6 @@ Env {
"dev": false,
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": false,
Expand Down
1 change: 0 additions & 1 deletion src/core/server/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export interface CliArgs {
basePath: boolean;
optimize: boolean;
open: boolean;
pluginPath?: string[];
}

export class Env {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ Array [
"dev": true,
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": true,
"repl": false,
"silent": false,
Expand Down Expand Up @@ -68,7 +67,6 @@ Array [
"dev": true,
"open": false,
"optimize": false,
"pluginPath": undefined,
"quiet": false,
"repl": false,
"silent": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,11 @@ export class LegacyObjectToConfigAdapter extends ObjectToConfigAdapter {
}

private static transformPlugins(configValue: Record<string, any>) {
// This property is the only one we use from the existing `plugins` config node
// since `scanDirs` and `paths` aren't respected by new platform plugin discovery.
// These properties are the only ones we use from the existing `plugins` config node
// since `scanDirs` isn't respected by new platform plugin discovery.
return {
initialize: configValue.initialize,
paths: configValue.paths,
};
}

Expand Down
6 changes: 4 additions & 2 deletions src/core/server/plugins/discovery/plugin_discovery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,13 @@ test('properly iterates through plugin search locations', async () => {

const env = Env.createDefault(
getEnvOptions({
cliArgs: { envName: 'development', pluginPath: [TEST_EXTRA_PLUGIN_PATH] },
cliArgs: { envName: 'development' },
})
);
const configService = new ConfigService(
new BehaviorSubject<Config>(new ObjectToConfigAdapter({})),
new BehaviorSubject<Config>(
new ObjectToConfigAdapter({ plugins: { paths: [TEST_EXTRA_PLUGIN_PATH] } })
),
env,
logger
);
Expand Down
6 changes: 3 additions & 3 deletions src/core/server/plugins/discovery/plugins_discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,16 @@ export function discover(config: PluginsConfig, coreContext: CoreContext) {
const log = coreContext.logger.get('plugins-discovery');
log.debug('Discovering plugins...');

if (config.knownPluginPaths.length) {
if (config.additionalPluginPaths.length) {
log.warn(
`Explicit plugin paths ${JSON.stringify(
config.knownPluginPaths
config.additionalPluginPaths
)} are only supported in development. Relative imports will not work in production.`
);
}

const discoveryResults$ = merge(
from(config.knownPluginPaths),
from(config.additionalPluginPaths),
processPluginSearchPaths$(config.pluginSearchPaths, log)
).pipe(
mergeMap(pluginPathOrError => {
Expand Down
13 changes: 9 additions & 4 deletions src/core/server/plugins/plugins_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ import { Env } from '../config';

const pluginsSchema = schema.object({
initialize: schema.boolean({ defaultValue: true }),

/**
* Defines an array of directories where another plugin should be loaded from.
* Should only be used in a development environment.
*/
paths: schema.arrayOf(schema.string(), { defaultValue: [] }),
});

type PluginsConfigType = TypeOf<typeof pluginsSchema>;
Expand All @@ -43,13 +49,12 @@ export class PluginsConfig {
/**
* Defines directories where a known plugin exists.
*/
public readonly knownPluginPaths: ReadonlyArray<string>;
public readonly additionalPluginPaths: ReadonlyArray<string>;

constructor(config: PluginsConfigType, env: Env) {
this.initialize = config.initialize;
this.pluginSearchPaths = env.pluginSearchPaths;
// Only allow custom pluginPaths in dev.
this.knownPluginPaths =
env.cliArgs.envName === 'development' ? env.cliArgs.pluginPath || [] : [];
// Only allow custom plugin paths in dev.
this.additionalPluginPaths = env.mode.dev ? config.paths : [];
}
}
2 changes: 1 addition & 1 deletion src/core/server/plugins/plugins_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ test('`setup` properly invokes `discover` and ignores non-critical errors.', asy
expect(mockDiscover).toHaveBeenCalledTimes(1);
expect(mockDiscover).toHaveBeenCalledWith(
{
additionalPluginPaths: [],
initialize: true,
knownPluginPaths: [],
pluginSearchPaths: [
resolve(process.cwd(), 'src', 'plugins'),
resolve(process.cwd(), 'plugins'),
Expand Down

0 comments on commit 0ce12c5

Please sign in to comment.