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

fix: schema validation errors disguised as warnings #1640

Merged
merged 3 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion jest/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export async function runUntil(
...options,
});

spawnPromise.stderr.pipe(
spawnPromise.stderr?.pipe(
new Writable({
write(chunk: any, _encoding: string, callback: () => void) {
const output = chunk.toString('utf8');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ Object {
}
`;

exports[`should not skip packages that have invalid configuration (to avoid breaking users): dependencies config 1`] = `
Object {
"react-native": Object {
"name": "react-native",
"platforms": Object {},
"root": "<<REPLACED>>/node_modules/react-native",
},
}
`;

exports[`should not skip packages that have invalid configuration (to avoid breaking users): logged warning 1`] = `"warn Package react-native contains invalid configuration: \\"dependency.invalidProperty\\" is not allowed. Please verify it's properly linked using \\"react-native config\\" command and contact the package maintainers about this."`;

exports[`should read a config of a dependency and use it to load other settings 1`] = `
Object {
"name": "react-native-test",
Expand Down Expand Up @@ -125,10 +137,6 @@ Object {
}
`;

exports[`should skip packages that have invalid configuration: dependencies config 1`] = `Object {}`;

exports[`should skip packages that have invalid configuration: logged warning 1`] = `"warn Package react-native has been ignored because it contains invalid configuration. Reason: \\"dependency.invalidProperty\\" is not allowed"`;

exports[`supports dependencies from user configuration with custom build type 1`] = `
Object {
"name": "react-native-test",
Expand Down
6 changes: 4 additions & 2 deletions packages/cli-config/src/__tests__/index-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ test('should load commands from "react-native-foo" and "react-native-bar" packag
expect(commands).toMatchSnapshot();
});

test('should skip packages that have invalid configuration', () => {
test('should not skip packages that have invalid configuration (to avoid breaking users)', () => {
process.env.FORCE_COLOR = '0'; // To disable chalk
DIR = getTempDirectory('config_test_skip');
writeFiles(DIR, {
Expand All @@ -208,7 +208,9 @@ test('should skip packages that have invalid configuration', () => {
}`,
});
const {dependencies} = loadConfig(DIR);
expect(dependencies).toMatchSnapshot('dependencies config');
expect(removeString(dependencies, DIR)).toMatchSnapshot(
'dependencies config',
);
expect(spy.mock.calls[0][0]).toMatchSnapshot('logged warning');
});

Expand Down
24 changes: 3 additions & 21 deletions packages/cli-config/src/loadConfig.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import path from 'path';
import chalk from 'chalk';
import {
UserDependencyConfig,
ProjectConfig,
Expand All @@ -8,8 +7,6 @@ import {
Config,
} from '@react-native-community/cli-types';
import {
logger,
inlineString,
findProjectRoot,
resolveNodeModuleDir,
} from '@react-native-community/cli-tools';
Expand Down Expand Up @@ -101,24 +98,9 @@ function loadConfig(projectRoot: string = findProjectRoot()): Config {
const localDependencyRoot =
userConfig.dependencies[dependencyName] &&
userConfig.dependencies[dependencyName].root;
let root: string;
let config: UserDependencyConfig;
try {
root =
localDependencyRoot ||
resolveNodeModuleDir(projectRoot, dependencyName);
config = readDependencyConfigFromDisk(root);
} catch (error) {
logger.warn(
inlineString(`
Package ${chalk.bold(
dependencyName,
)} has been ignored because it contains invalid configuration.

Reason: ${chalk.dim(error.message)}`),
);
return acc;
}
let root =
localDependencyRoot || resolveNodeModuleDir(projectRoot, dependencyName);
let config = readDependencyConfigFromDisk(root, dependencyName);

const isPlatform = Object.keys(config.platforms).length > 0;

Expand Down
17 changes: 15 additions & 2 deletions packages/cli-config/src/readConfigFromDisk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
UserConfig,
UserDependencyConfig,
} from '@react-native-community/cli-types';
import {logger, inlineString} from '@react-native-community/cli-tools';
import chalk from 'chalk';

/**
* Places to look for the configuration file.
Expand Down Expand Up @@ -38,6 +40,7 @@ export function readConfigFromDisk(rootFolder: string): UserConfig {
*/
export function readDependencyConfigFromDisk(
rootFolder: string,
dependencyName: string,
): UserDependencyConfig {
const explorer = cosmiconfig('react-native', {
stopDir: rootFolder,
Expand All @@ -47,10 +50,20 @@ export function readDependencyConfigFromDisk(
const searchResult = explorer.searchSync(rootFolder);
const config = searchResult ? searchResult.config : emptyDependencyConfig;

const result = schema.dependencyConfig.validate(config);
const result = schema.dependencyConfig.validate(config, {abortEarly: false});

if (result.error) {
throw new JoiError(result.error);
const validationError = new JoiError(result.error);
logger.warn(
inlineString(`
Package ${chalk.bold(
dependencyName,
)} contains invalid configuration: ${chalk.bold(
validationError.message,
)}.

Please verify it's properly linked using "react-native config" command and contact the package maintainers about this.`),
);
}

return result.value as UserDependencyConfig;
Expand Down
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2616,7 +2616,7 @@
"@types/node" "*"
form-data "^3.0.0"

"@types/node@*", "@types/node@^12.0.0":
"@types/node@*", "@types/node@^12.0.0", "@types/node@^17.0.35":
version "12.20.47"
resolved "https://registry.yarnpkg.com/@types/node/-/node-12.20.47.tgz#ca9237d51f2a2557419688511dab1c8daf475188"
integrity sha512-BzcaRsnFuznzOItW1WpQrDHM7plAa7GIDMZ6b5pnMbkqEtM/6WCOhvZar39oeMQP79gwvFUWjjptE7/KGcNqFg==
Expand Down