Skip to content

Commit

Permalink
v1 ready
Browse files Browse the repository at this point in the history
  • Loading branch information
shaharkazaz committed Oct 28, 2024
1 parent 9f9facd commit 6834597
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 70 deletions.
24 changes: 20 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,24 @@ To get started, install Letify CLI:
npm i -D @jsverse/letify
```

Then, analyze your HTML files to identify redundant subscriptions:
Then, run the command:
```bash
npx letify analyze 'a/b.html' 'c/**/*.html' ...
npx letify [analyze|fix] 'a/b.html' 'c/**/*.html' ...
```

* `analyze`: Identifies duplicate subscriptions in the specified files and generates a report.
* `fix`: Identifies duplicate subscriptions and replace duplications with a single `@let` declaration at the beginning of the template.

#### CI / Lint-
Letify will return an error exit code if any duplicate subscriptions are detected in the specified files.
It can be seamlessly integrated into your [lint-staged](https://github.com/lint-staged/lint-staged) or CI workflows to
prevent duplicate subscriptions from being committed.

_Note_: Letify ignores commented code and does not analyze it.
#### Usage notes
* Letify ignores commented code and does not analyze it.
* Keyed reads (`data[prop] | async`) and function calls with arguments (`myMethod(value, ...) | async`) are currently not supported.
* You'll need Angular `>=18.1` to use the `@let` syntax, if you are using an older version, run the `analyze` command and
use alternatives to reuse your subscriptions.

### Options

Expand All @@ -49,4 +56,13 @@ _Note_: Letify ignores commented code and does not analyze it.
* `list`: Outputs a simple list of suggestions.
* `json`: Provides a JSON report for programmatic use.
* `-o, --open`: Automatically opens the HTML report once generated (default `true`.
* `--verify-convention`: Checks that stream names (observables) in the templates follow the convention of ending with a `$` sign (default: `false`).
* `--verify-convention` (default: `false`): Checks that stream names (observables) in the templates follow the convention of ending with a `$` sign.
* `--variable-suffix` (default: `value`): Adds a suffix to the declared variable in `fix` mode, mainly to avoid collisions.

### Debugging

You can extend the default logs by setting the `DEBUG` environment variable:
```bash
DEBUG=letify:* npx letify ...
```
Supported namespaces: `letify:*|letify:fix`.
15 changes: 9 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
"dependencies": {
"@angular/compiler": ">=17.0.0",
"@jsverse/utils": "1.0.0",
"cheerio": "^1.0.0",
"commander": "^12.1.0",
"glob": "^11.0.0",
"open": "^10.1.0",
"ora": "^8.1.0",
"tslib": "^2.3.0"
"cheerio": "1.0.0",
"cli-table3": "0.6.5",
"commander": "12.1.0",
"debug": "4.3.7",
"glob": "11.0.0",
"open": "10.1.0",
"ora": "8.1.0",
"tslib": "2.3.0"
},
"devDependencies": {
"@eslint/js": "^9.8.0",
Expand All @@ -35,6 +37,7 @@
"@swc-node/register": "~1.9.1",
"@swc/core": "~1.5.7",
"@swc/helpers": "~0.5.11",
"@types/debug": "^4.1.12",
"@types/node": "18.16.9",
"@vitest/coverage-v8": "^1.0.4",
"@vitest/ui": "^1.3.1",
Expand Down
5 changes: 5 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ program
program
.option('-o, --open', 'Open the report (in case of html reporter)', false)
.option('--verify-convention', 'Verify stream names are ending with $', false)
.option(
'--variable-suffix',
'Adds a suffix to the declared variable in `fix` mode, mainly to avoid collisions',
'value'
)
.addOption(
new Option('-r, --reporter <type>', 'Reporter type')
.choices(['list', 'json', 'html'])
Expand Down
82 changes: 64 additions & 18 deletions src/lib/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,31 @@ import {
} from '@angular/compiler';
import { underline } from './reporters/list-reporter';
import { reporters } from './reporters';
import { isCI, loader } from './utils';
import { isCI, loader, verifyStreamName } from './utils';
import {
AstPipeCollector,
PropertyNameResolver,
TmplPipeCollector,
} from '@jsverse/utils';
import { fixTemplate } from './fix-template';

export function analyze({ options, templateFiles }: CmdConfig) {
const templatesRequiringReview = [];
export async function analyze({ options, templateFiles, cmd }: CmdConfig) {
const templatesRequiringReview: Result['templatesRequiringReview'] = [];
const fileAnalysis: Result['fileAnalysis'] = {};

for (const file of templateFiles) {
loader.start(`Analyzing ${path.basename(file)}`);
const content = readFile(file);
const content = await readFile(file);
const noComments = content.replace(/<!--[\s\S]*?-->/g, '');
const asyncPipesCount = noComments.match(/\|\s*async/g)?.length ?? 0;

if (!asyncPipesCount) continue;

const parsedTemplate = ngParseTemplate(content, file);
const parsedTemplate = ngParseTemplate(noComments, file, {
preserveWhitespaces: false,
preserveLineEndings: false,
preserveSignificantWhitespace: false,
});
if (parsedTemplate.errors?.length > 0) {
loader.fail(`@angular/compiler failed to parse ${file}, skipping...`);
continue;
Expand All @@ -42,36 +47,60 @@ export function analyze({ options, templateFiles }: CmdConfig) {
.get('async')
?.map((pipe) => {
const resolver = new PropertyNameResolver();
resolver.visit(pipe.exp, {});
resolver.visit(pipe.node.exp, {});

return resolver.fullPropertyPath;
return {
...pipe,
name: resolver.path,
};
})
.forEach((name) => {
analysisResult[name] ??= 0;
analysisResult[name]++;
.forEach(({ name, ...rest }) => {
analysisResult[name] ??= { count: 0, pipes: [] };
analysisResult[name].count++;
analysisResult[name].pipes.push(rest);
});

// This means we have cases where the word 'async' is used but, we didn't extract the usage
const totalExtractedPipes = Object.values(analysisResult).reduce(
(acc, count) => acc + count,
(acc, { count }) => acc + count,
0
);
if (totalExtractedPipes === 0 || totalExtractedPipes !== asyncPipesCount) {
templatesRequiringReview.push(path.resolve(file));
templatesRequiringReview.push({
file: path.resolve(file),
pipeCount: asyncPipesCount,
extractedCount: totalExtractedPipes,
});
continue;
}

const dupSubs = Object.entries(analysisResult).filter(
([, count]) => count > 1
([, { count }]) => count > 1
);
if (dupSubs.length > 0) {
fileAnalysis[file] = dupSubs.map(([name, count]) => ({
let variablesAdded: Awaited<ReturnType<typeof fixTemplate>> = {};

if (cmd === 'fix') {
const parsedFile =
parsedTemplate.nodes[0].sourceSpan.start.file.content;
variablesAdded = await fixTemplate({
variableSuffix: options.variableSuffix,
content,
dupSubs,
parsedFile,
file,
});
}

fileAnalysis[file] = dupSubs.map(([name, { count }]) => ({
name,
count,
isCompliant: options.verifyConvention ? verifyStreamName(name) : true,
variableName: variablesAdded[name],
}));
}
}

loader.succeed(
`${templateFiles.length.toLocaleString()} files were analyzed`
);
Expand All @@ -83,15 +112,36 @@ export function analyze({ options, templateFiles }: CmdConfig) {
const hasDuplications = duplicateSubCount > 0;

if (hasDuplications) {
let totalSubs = 0;
let reducedSubs = 0;

for (const occurrences of Object.values(fileAnalysis)) {
for (const { count } of occurrences) {
totalSubs += count;
reducedSubs++;
}
}
const savedSubs = totalSubs - reducedSubs;
console.log(
`❗ ${duplicateSubCount.toLocaleString()} files had duplicated subscriptions in them`
);
const asPercent = Math.ceil((savedSubs / totalSubs) * 100);
const msg = cmd === 'fix' ? '✨ Letify reduced' : '❗ You can reduce';
console.log(
`${msg} your subscriptions by ${asPercent}% (from ${totalSubs.toLocaleString()} to ${reducedSubs.toLocaleString()})`
);
if (cmd === 'fix') {
console.log(
'\nMake sure you manually review the changes, letify is not perfect'
);
}
} else {
console.log(`✨ No duplicate subscriptions were found`);
}
console.log('');

const reporter = new reporters[options.reporter]({
cmd,
result: {
templatesRequiringReview,
filesScanned: templateFiles.length,
Expand All @@ -105,7 +155,3 @@ export function analyze({ options, templateFiles }: CmdConfig) {

process.exit(hasDuplications ? 1 : 0);
}

function verifyStreamName(name: string) {
return name.endsWith('$');
}
64 changes: 64 additions & 0 deletions src/lib/fix-template.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { writeFile } from 'node:fs/promises';
import { escapeRegExp, toCamelCase, verifyStreamName } from './utils';
import { AnalysisResult } from './types';
import { PropertyNameResolver } from '@jsverse/utils';
import debug from 'debug';

export async function fixTemplate({
content,
dupSubs,
file,
parsedFile,
variableSuffix,
}: {
content: string;
dupSubs: [string, AnalysisResult[string]][];
file: string;
parsedFile: string;
variableSuffix: string;
}) {
let updatedTemplate = content;
const variables: Record<string, string> = {};
for (const [pipeName, { pipes }] of dupSubs) {
const resolver = new PropertyNameResolver({
// we want the right side of the expression as it's what we will pass to the async pipe
binaryNavigator: (ast) => ast.right,
});
const [pipe] = pipes;
resolver.visit(pipe.node.exp, {});
const name = resolver.segments.filter((p) => p !== 'asObservable').pop();
const letName = toCamelCase(
`${verifyStreamName(name) ? name.slice(0, -1) : name}-${variableSuffix}`
);

const regex = `${escapeRegExp(resolver.path)}[^\\s]*\\s?\\|\\s?async`;
const asyncPipeReg = new RegExp(regex).exec(parsedFile)!;

if (!asyncPipeReg) {
if (debug.enabled('letify:fix')) {
console.log({ file, path: resolver.path, regex });
}
variables[pipeName] = '⚠️ Not created, unable to resolve pipe expression';
continue;
}

const [asyncPipe] = asyncPipeReg;

if (/\(.+\)/.test(asyncPipe)) {
if (debug.enabled('letify:fix')) {
console.log({ file, path: resolver.path, regex, asyncPipe });
}
variables[pipeName] = '⚠️ Not created, arguments are not supported';
continue;
}
updatedTemplate = updatedTemplate.replace(
new RegExp(escapeRegExp(asyncPipe), 'g'),
letName
);
updatedTemplate = `@let ${letName} = ${asyncPipe};\n${updatedTemplate}`;
variables[pipeName] = letName;
}
await writeFile(file, updatedTemplate);

return variables;
}
5 changes: 0 additions & 5 deletions src/lib/fix-templates.ts

This file was deleted.

10 changes: 3 additions & 7 deletions src/lib/letify.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { glob } from 'glob';
import ora from 'ora';
import { analyze } from './analyze';
import { fixTemplates } from './fix-templates';

export interface Options {
inputs: string[];
reporter: 'list' | 'json' | 'html';
open: boolean;
verifyConvention: boolean;
variableSuffix: string;
}

export function letify(cmd: 'analyze' | 'fix', options: Options) {
export async function letify(cmd: 'analyze' | 'fix', options: Options) {
const templateFiles = options.inputs
.map((p) =>
glob.sync(p, {
Expand All @@ -25,9 +25,5 @@ export function letify(cmd: 'analyze' | 'fix', options: Options) {
return;
}

if (cmd === 'analyze') {
analyze({ options, templateFiles });
} else {
fixTemplates({ options, templateFiles });
}
await analyze({ options, templateFiles, cmd });
}
Loading

0 comments on commit 6834597

Please sign in to comment.