Skip to content
This repository was archived by the owner on Sep 6, 2018. It is now read-only.

Multiple call to import #14

Merged
merged 12 commits into from
Dec 26, 2017
Merged
Show file tree
Hide file tree
Changes from 10 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
15 changes: 10 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
before_install:
- nvm install 8

install:
- npm install

language: node_js
node_js:
- "node"
- "6"
script:
- npm test

script:
- npm run compile
- npm run lint
- npm run jest
37 changes: 21 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,37 +30,42 @@ By default, `mcritic` will recursively search the current and parent folders for

```json
{
"callToImportRegex": "(\\S+)",
"callToImportReplace": "{$1|param}"
"callToImport": [
{
"regex": "(\\S+)",
"replace": "{$1|param}"
}
]
}
```

Here are the currently supported **settings**:

### `callToImportRegex`
### `callToImport`

This property is used to provide matches for `callToImportReplace`. This should be a standard javascript regex that contains capture groups to be referenced in `callToImportReplace`.
This property is used to provide a list of replacement configurations. Each list item must contain two properties:

|Type|Default|
|----|-------|
|string|`"(.*)"`|
- `regex`

This property is used to provide matches for `replace`. This should be a standard javascript regex that contains capture groups to be referenced in `replace`.

- `replace`

### `callToImportReplace`
This property uses match groups defined in `regex` to translate a component name in a soy template to its corresponding import name when validating their import.

This property uses match groups defined in `callToImportRegex` to translate a component name in a soy template to its corresponding import name when validating their import.
When referencing match groups from `regex`, interpolation should be in the form of `{$n}`, where `n` is the match group number. An example would be `"{$1}.js"`.

When referencing match groups from `callToImportRegex`, interpolation should be in the form of `{$n}`, where `n` is the match group number. An example would be `"{$1}.js"`.
Interpolations can also contain named string transformations delimited by a `|`. This transformation corresponds to the functions provided by [`change-case`](https://www.npmjs.com/package/change-case).

Interpolations can also contain named string transformations delimited by a `|`. This transformation corresponds to the functions provided by [`change-case`](https://www.npmjs.com/package/change-case).
Examples:
* `"{$1|lower|snake}.js"`
* `"{$2}-{$1}"`
* `"{$1|dot}.js"`

Examples:
* `"{$1|lower|snake}.js"`
* `"{$2}-{$1}"`
* `"{$1|dot}.js"`

|Type|Default|
|----|-------|
|string|`"{$1}"`|
|array|`"[{"regex": "(.*)", "replace": "{$1}"}]"`|

### `implicitParams`

Expand Down
9 changes: 5 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@
"engines": {
"node": ">=6.4.0"
},
"repository": "https://github.com/mthadley/metal-soy-critic",
"repository": "https://github.com/metal/metal-soy-critic",
"main": "./lib/index.js",
"files": [
"lib"
],
"scripts": {
"lint": "tslint src/**/*.ts",
"test": "npm run compile && jest",
"compile": "tsc",
"prepublish": "npm run compile"
"jest": "jest",
"lint": "tslint src/**/*.ts",
"prepublish": "npm run compile",
"test": "npm run compile && jest"
},
"jest": {
"moduleFileExtensions": [
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/__snapshots__/index.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exports[`cli should accept an ignore glob 1`] = `
`;

exports[`cli should fail with invalid config file 1`] = `
"Failed to read config: callToImportRegex is not a valid RegExp.
"Failed to read config: callToImport.regex \\"(\\\\S+\\" is not a valid RegExp.
"
`;

Expand Down
22 changes: 17 additions & 5 deletions src/__tests__/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,31 @@ describe('config', () => {
const config = Config.readConfig();

expect(config).toBeInstanceOf(Object);
expect(config.callToImportRegex).toBe('(.*)');
expect(config.callToImport).toEqual([{regex: '(.*)', replace: '{$1}'}]);
});

it('should return a configuration specified in a file', () => {
process.chdir('./test/fixtures/config');

const config = Config.readConfig();

expect(config.callToImportRegex).toBe('(\\S+)');
expect(config.callToImport).toEqual([{regex: '(\\S+)', replace: '{$1|param}'}]);
});

it('should read config files with a json extension', () => {
process.chdir('./test/fixtures/config-json');

const config = Config.readConfig();

expect(config.callToImportRegex).toBe('json');
expect(config.callToImport).toEqual([{regex: 'json', replace: '{$1|param}'}]);
});

it('should return a converted configuration specified in a file if is found in the old way', () => {
process.chdir('./test/fixtures/deprecated-config');

const config = Config.readConfig();

expect(config.callToImport).toEqual([{regex: '(\\S+)', replace: '{$1|param}'}]);
});
});

Expand All @@ -70,8 +78,12 @@ describe('config', () => {

it('should throw an Error if the config is invalid', () => {
const invalidConfig = {
callToImportRegex: '(asd',
callToImportReplace: 'bar',
callToImport: [
{
regex: '(asd',
replace: 'bar',
},
],
implicitParams: {},
};

Expand Down
50 changes: 40 additions & 10 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as chalk from 'chalk';
import * as fs from 'fs';
import * as path from 'path';
import * as process from 'process';
Expand All @@ -7,29 +8,40 @@ const CONFIG_FILE_NAMES = [
'.soycriticrc.json',
];

type CallToImportConfig = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Generally, it's better to prefer interfaces over type alias for this kind of thing.

regex: string;
replace: string;
}

export interface ImplicitParamsMap {
[nameOrRegex: string]: string | Array<string>
}

export interface Config {
callToImportRegex: string
callToImportReplace: string
callToImport: Array<CallToImportConfig>
implicitParams: ImplicitParamsMap
}

export const DEFAULT_CONFIG: Config = {
callToImportRegex: '(.*)',
callToImportReplace: '{$1}',
callToImport: [{regex: '(.*)',replace: '{$1}'}],
implicitParams: {}
};

export function validateConfig(config: Config): Config {
if (!isRegex(config.callToImportRegex)) {
throw new Error('callToImportRegex is not a valid RegExp.');
if (!Array.isArray(config.callToImport)) {
throw new Error('callToImport is not a valid config array.');
}

if (!isRegex(config.callToImportReplace)) {
throw new Error('callToImportReplace is not a valid replace string.');
for (let i=0; i < config.callToImport.length; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer for of over c-style loops:

for (const item of config.callToImport) {
  /* ... */
}

let callToImportItem = config.callToImport[i];

if (!isRegex(callToImportItem.regex)) {
throw new Error(`callToImport.regex "${callToImportItem.regex}" is not a valid RegExp.`);
}

if (!isRegex(callToImportItem.replace)) {
throw new Error(`callToImport.replace "${callToImportItem.replace}" is not a valid replace string.`);
}
}

for (const key in config.implicitParams) {
Expand All @@ -41,6 +53,22 @@ export function validateConfig(config: Config): Config {
return config;
}

export function convertConfig(config: any): Config {
if (config.callToImportRegex && config.callToImportReplace) {
config.callToImport = [
{
regex: config.callToImportRegex,
replace: config.callToImportReplace
}
];

console.log(chalk.yellow('CONFIG API HAS CHANGED, PLEASE UPDATE\n'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

💯

console.log('\tYour callToImport configuration is outdated, update it to new API.\n');
}

return config;
}

export function readConfig(): Config {
const filePath = getConfigFilePath();
let config = {};
Expand All @@ -50,7 +78,9 @@ export function readConfig(): Config {

config = JSON.parse(buffer.toString('utf8'));
}


config = convertConfig(config);

return validateConfig({...DEFAULT_CONFIG, ...config});
}

Expand Down Expand Up @@ -78,4 +108,4 @@ export function isRegex(regex: string): boolean {
return false;
}
return true;
}
}
19 changes: 15 additions & 4 deletions src/validate-call-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,30 @@ function getImportPaths(ast: T.Node): Array<string> {
jsTraverse(ast, {
ImportDeclaration(path) {
importPaths.push(path.node.source.value);
},
ImportSpecifier(path) {
importPaths.push(path.node.imported.name);
}
});

return importPaths;
}

export default function valdiateCallImports(soyContext: SoyContext, jsContext: JSContext, config: Config): Result {
const importNames = getImportPaths(jsContext.ast)
.map(importPath => path.parse(importPath).name);

const missingImports = getExternalSoyCalls(soyContext)
const missingImports = getExternalSoyCalls(soyContext)
.filter(name => {
name = transform(name, config.callToImportRegex, config.callToImportReplace);
return !importNames.find(importName => importName.includes(name));
for (let i=0; i < config.callToImport.length; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, try using a for of instead.

let transformedName = transform(name, config.callToImport[i].regex, config.callToImport[i].replace);

if (importNames.find(importName => importName.includes(transformedName))) {
return false;
}
}

return true;
});

if (missingImports.length) {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/CaseImport.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Component from 'metal-component';
import Soy from 'metal-soy';

import 'OtherComponent';
import {OtherSubComponent} from 'OtherComponent';
import templates from './CaseImport.soy';
import {Config} from 'metal-state';

Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/CaseImport.soy
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@
{call OtherComponent.render}
{param foo: 'bar' /}
{/call}

{call OtherSubComponent.render}
{param foo: 'bar' /}
{/call}
{/template}
8 changes: 6 additions & 2 deletions test/fixtures/config-json/.soycriticrc.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{
"callToImportRegex": "json",
"callToImportReplace": "{$1|param}"
"callToImport": [
{
"regex": "json",
"replace": "{$1|param}"
}
]
}
8 changes: 6 additions & 2 deletions test/fixtures/config/.soycriticrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{
"callToImportRegex": "(\\S+)",
"callToImportReplace": "{$1|param}"
"callToImport": [
{
"regex": "(\\S+)",
"replace": "{$1|param}"
}
]
}
4 changes: 4 additions & 0 deletions test/fixtures/deprecated-config/.soycriticrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"callToImportRegex": "(\\S+)",
"callToImportReplace": "{$1|param}"
}
8 changes: 6 additions & 2 deletions test/fixtures/invalid-config/.soycriticrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{
"callToImportRegex": "(\\S+",
"callToImportReplace": "{$1|param}"
"callToImport": [
{
"regex": "(\\S+",
"replace": "{$1|param}"
}
]
}