feat(core): make processing typescript dependencies in rust the default (#18398)

This commit is contained in:
Jason Jean 2023-08-01 18:19:52 -04:00 committed by GitHub
parent 3ae657cad3
commit 842cf7aa11
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 126 additions and 171 deletions

View File

@ -384,7 +384,6 @@ describe('Nx Affected and Graph Tests', () => {
target: mylib, target: mylib,
type: 'static', type: 'static',
}, },
{ source: myapp, target: mylib2, type: 'dynamic' },
], ],
[myappE2e]: [ [myappE2e]: [
{ {

View File

@ -1,7 +1,6 @@
use std::collections::HashMap; use std::collections::{HashMap, HashSet};
use std::fmt::Debug; use std::fmt::Debug;
use std::path::Path; use std::path::Path;
use std::rc::Rc;
use std::sync::Arc; use std::sync::Arc;
use std::time::Instant; use std::time::Instant;
@ -9,7 +8,8 @@ use rayon::prelude::*;
use tracing::debug; use tracing::debug;
use tracing::trace; use tracing::trace;
use swc_common::{BytePos, SourceFile, SourceMap, Spanned}; use swc_common::comments::SingleThreadedComments;
use swc_common::{BytePos, SourceMap, Spanned};
use swc_ecma_ast::EsVersion::EsNext; use swc_ecma_ast::EsVersion::EsNext;
use swc_ecma_parser::error::Error; use swc_ecma_parser::error::Error;
use swc_ecma_parser::lexer::Lexer; use swc_ecma_parser::lexer::Lexer;
@ -449,6 +449,8 @@ fn process_file((source_project, file_path): (&String, &String)) -> Option<Impor
.load_file(Path::new(file_path)) .load_file(Path::new(file_path))
.unwrap(); .unwrap();
let comments = SingleThreadedComments::default();
let tsx = file_path.ends_with(".tsx") || file_path.ends_with(".jsx"); let tsx = file_path.ends_with(".tsx") || file_path.ends_with(".jsx");
let lexer = Lexer::new( let lexer = Lexer::new(
Syntax::Typescript(TsConfig { Syntax::Typescript(TsConfig {
@ -460,14 +462,14 @@ fn process_file((source_project, file_path): (&String, &String)) -> Option<Impor
}), }),
EsNext, EsNext,
(&*cm).into(), (&*cm).into(),
None, Some(&comments),
); );
// State // State
let mut state = State::new(lexer); let mut state = State::new(lexer);
let mut static_import_expressions: Vec<String> = vec![]; let mut static_import_expressions: Vec<(String, BytePos)> = vec![];
let mut dynamic_import_expressions: Vec<String> = vec![]; let mut dynamic_import_expressions: Vec<(String, BytePos)> = vec![];
loop { loop {
let current_token = state.next(); let current_token = state.next();
@ -477,6 +479,8 @@ fn process_file((source_project, file_path): (&String, &String)) -> Option<Impor
break; break;
} }
let mut pos: Option<BytePos> = None;
if let Some(current) = &current_token { if let Some(current) = &current_token {
let word = match &current.token { let word = match &current.token {
Token::Word(w) => w, Token::Word(w) => w,
@ -487,35 +491,29 @@ fn process_file((source_project, file_path): (&String, &String)) -> Option<Impor
let import = match word { let import = match word {
// This is an import keyword // This is an import keyword
Keyword(keyword) if *keyword == Import => { Keyword(keyword) if *keyword == Import => {
if is_code_ignored(&cm, current.span.lo) { pos = Some(current.span.lo);
continue;
}
find_specifier_in_import(&mut state) find_specifier_in_import(&mut state)
} }
Keyword(keyword) if *keyword == Export => { Keyword(keyword) if *keyword == Export => {
if is_code_ignored(&cm, current.span.lo) { pos = Some(current.span.lo);
continue;
}
find_specifier_in_export(&mut state) find_specifier_in_export(&mut state)
} }
Ident(ident) if ident == "require" => { Ident(ident) if ident == "require" => {
if is_code_ignored(&cm, current.span.lo) { pos = Some(current.span.lo);
continue;
}
find_specifier_in_require(&mut state) find_specifier_in_require(&mut state)
} }
_ => None, _ => None,
}; };
if let Some((specifier, import_type)) = import { if let Some((specifier, import_type)) = import {
let pos = pos.expect("Always exists when there is an import");
match import_type { match import_type {
ImportType::Static => { ImportType::Static => {
static_import_expressions.push(specifier); static_import_expressions.push((specifier, pos));
} }
ImportType::Dynamic => { ImportType::Dynamic => {
dynamic_import_expressions.push(specifier); dynamic_import_expressions.push((specifier, pos));
} }
} }
} }
@ -539,11 +537,42 @@ fn process_file((source_project, file_path): (&String, &String)) -> Option<Impor
errs.clear(); errs.clear();
} }
if file_path == "nx-dev/ui-markdoc/src/lib/tags/graph.component.tsx" { // Create a HashMap of comments by the lines where they end
trace!("{:?}", static_import_expressions); let (leading_comments, _) = comments.take_all();
trace!("{:?}", dynamic_import_expressions); let mut lines_with_nx_ignore_comments: HashSet<usize> = HashSet::new();
let leading_comments = leading_comments.borrow();
for (_, comments) in leading_comments.iter() {
for comment in comments {
let comment_text = comment.text.trim();
if comment_text.contains("nx-ignore-next-line") {
let line_where_comment_ends = cm
.lookup_line(comment.span.hi)
.expect("Comments end on a line");
lines_with_nx_ignore_comments.insert(line_where_comment_ends);
}
}
} }
let code_is_not_ignored = |(specifier, pos): (String, BytePos)| {
let line_with_code = cm.lookup_line(pos).expect("All code is on a line");
if lines_with_nx_ignore_comments.contains(&(line_with_code - 1)) {
None
} else {
Some(specifier)
}
};
let static_import_expressions = static_import_expressions
.into_iter()
.filter_map(code_is_not_ignored)
.collect();
let dynamic_import_expressions = dynamic_import_expressions
.into_iter()
.filter_map(code_is_not_ignored)
.collect();
Some(ImportResult { Some(ImportResult {
file: file_path.clone(), file: file_path.clone(),
source_project: source_project.clone(), source_project: source_project.clone(),
@ -552,22 +581,6 @@ fn process_file((source_project, file_path): (&String, &String)) -> Option<Impor
}) })
} }
fn is_code_ignored(cm: &Rc<SourceFile>, pos: BytePos) -> bool {
let line_with_dep = cm.lookup_line(pos).expect("The dep is on a line");
if line_with_dep == 0 {
return false;
}
if let Some(line_before_dep) = cm.get_line(line_with_dep - 1) {
let trimmed_line = line_before_dep.trim();
if trimmed_line == "// nx-ignore-next-line" || trimmed_line == "/* nx-ignore-next-line */" {
return true;
}
}
false
}
#[napi] #[napi]
fn find_imports(project_file_map: HashMap<String, Vec<String>>) -> Vec<ImportResult> { fn find_imports(project_file_map: HashMap<String, Vec<String>>) -> Vec<ImportResult> {
enable_logger(); enable_logger();
@ -589,6 +602,55 @@ mod find_imports {
use assert_fs::TempDir; use assert_fs::TempDir;
use swc_common::comments::NoopComments; use swc_common::comments::NoopComments;
#[test]
fn should_not_include_ignored_imports() {
let temp_dir = TempDir::new().unwrap();
temp_dir
.child("test.ts")
.write_str(
r#"
// nx-ignore-next-line
import 'a';
/* nx-ignore-next-line */
import 'a1';
/* nx-ignore-next-line */
import 'a2';
/**
* nx-ignore-next-line
*/
import 'a3';
/*
nx-ignore-next-line
*/
import 'a4'; import 'a5';
/* prettier-ignore */ /* nx-ignore-next-line */
import 'a4'; import 'a5';
/* nx-ignore-next-line */ /* prettier-ignore */
import 'a4'; import 'a5';
"#,
)
.unwrap();
let test_file_path = temp_dir.display().to_string() + "/test.ts";
let results = find_imports(HashMap::from([(
String::from("a"),
vec![test_file_path.clone()],
)]));
let result = results.get(0).unwrap();
assert!(result.static_import_expressions.is_empty());
assert!(result.dynamic_import_expressions.is_empty());
}
#[test] #[test]
fn should_find_imports() { fn should_find_imports() {
let temp_dir = TempDir::new().unwrap(); let temp_dir = TempDir::new().unwrap();

View File

@ -24,8 +24,7 @@ export function buildExplicitDependencies(
// to be able to use at least 2 workers (1 worker per CPU and // to be able to use at least 2 workers (1 worker per CPU and
// 1 CPU for the main thread) // 1 CPU for the main thread)
if ( if (
(process.env.NX_NATIVE_TS_DEPS && process.env.NX_NATIVE_TS_DEPS !== 'false' ||
process.env.NX_NATIVE_TS_DEPS !== 'false') ||
jsPluginConfig.analyzeSourceFiles === false || jsPluginConfig.analyzeSourceFiles === false ||
totalNumOfFilesToProcess < 100 || totalNumOfFilesToProcess < 100 ||
getNumberOfWorkers() <= 2 getNumberOfWorkers() <= 2

View File

@ -36,7 +36,7 @@ describe('explicit project dependencies', () => {
}); });
const res = buildExplicitTypeScriptDependencies( const res = buildExplicitTypeScriptDependencies(
builder.graph, builder.getUpdatedProjectGraph(),
ctx.filesToProcess ctx.filesToProcess
); );
@ -47,12 +47,6 @@ describe('explicit project dependencies', () => {
targetProjectName: 'proj2', targetProjectName: 'proj2',
type: 'static', type: 'static',
}, },
{
sourceProjectName,
sourceProjectFile: 'libs/proj/index.ts',
targetProjectName: 'proj3a',
type: 'dynamic',
},
{ {
sourceProjectName, sourceProjectName,
sourceProjectFile: 'libs/proj/index.ts', sourceProjectFile: 'libs/proj/index.ts',
@ -65,6 +59,12 @@ describe('explicit project dependencies', () => {
targetProjectName: 'npm:npm-package', targetProjectName: 'npm:npm-package',
type: 'static', type: 'static',
}, },
{
sourceProjectName,
sourceProjectFile: 'libs/proj/index.ts',
targetProjectName: 'proj3a',
type: 'dynamic',
},
]); ]);
}); });
@ -380,6 +380,19 @@ describe('explicit project dependencies', () => {
nx-ignore-next-line */ nx-ignore-next-line */
import { foo } from '@proj/proj4ab'; import { foo } from '@proj/proj4ab';
/*
nx-ignore-next-line */
import { foo } from '@proj/proj4ab'; import { foo } from '@proj/project-3';
/* eslint-ignore */ /* nx-ignore-next-line */
import { foo } from '@proj/proj4ab'; import { foo } from '@proj/project-3';
const obj = {
// nx-ignore-next-line
a: import('@proj/proj4ab')
}
`, `,
}, },
], ],
@ -468,123 +481,6 @@ describe('explicit project dependencies', () => {
expect(res).toEqual([]); expect(res).toEqual([]);
}); });
}); });
/**
* In version 8, Angular deprecated the loadChildren string syntax in favor of using dynamic imports, but it is still
* fully supported by the framework:
*
* https://angular.io/guide/deprecations#loadchildren-string-syntax
*/
describe('legacy Angular loadChildren string syntax', () => {
it('should build explicit dependencies for legacy Angular loadChildren string syntax', async () => {
const sourceProjectName = 'proj';
const { ctx, builder } = await createVirtualWorkspace({
sourceProjectName,
sourceProjectFiles: [
{
path: 'libs/proj/file-1.ts',
content: `
const a = { loadChildren: '@proj/proj4ab#a' };
`,
},
{
path: 'libs/proj/file-2.ts',
content: `
const routes: Routes = [{
path: 'lazy',
loadChildren: '@proj/project-3#LazyModule',
}];
`,
},
/**
* TODO: This case, where a no subsitution template literal is used, is not working
*/
// {
// path: 'libs/proj/no-substitution-template-literal.ts',
// content: `
// const a = {
// loadChildren: \`@proj/my-second-proj\`
// };
// `,
// },
],
});
const res = buildExplicitTypeScriptDependencies(
builder.graph,
ctx.filesToProcess
);
expect(res).toEqual([
{
sourceProjectName,
sourceProjectFile: 'libs/proj/file-1.ts',
targetProjectName: 'proj4ab',
type: 'dynamic',
},
{
sourceProjectName,
sourceProjectFile: 'libs/proj/file-2.ts',
targetProjectName: 'proj3a',
type: 'dynamic',
},
]);
});
it('should not build explicit dependencies when nx-ignore-next-line comments are present', async () => {
const sourceProjectName = 'proj';
const { ctx, builder } = await createVirtualWorkspace({
sourceProjectName,
sourceProjectFiles: [
{
path: 'libs/proj/file-1.ts',
content: `
const a = {
// nx-ignore-next-line
loadChildren: '@proj/proj4ab#a'
};
`,
},
/**
* TODO: This case, where a multi-line comment is used, is not working
*/
// {
// path: 'libs/proj/file-2.ts',
// content: `
// const a = {
// /* nx-ignore-next-line */
// loadChildren: '@proj/proj4ab#a'
// };
// `,
// },
/**
* TODO: These cases, where loadChildren is on the same line as the variable declaration, are not working
*/
// {
// path: 'libs/proj/file-3.ts',
// content: `
// // nx-ignore-next-line
// const a = { loadChildren: '@proj/proj4ab#a' };
// `,
// },
// {
// path: 'libs/proj/file-4.ts',
// content: `
// /* nx-ignore-next-line */
// const a = { loadChildren: '@proj/proj4ab#a' };
// `,
// },
],
});
const res = buildExplicitTypeScriptDependencies(
builder.graph,
ctx.filesToProcess
);
expect(res).toEqual([]);
});
});
}); });
interface VirtualWorkspaceConfig { interface VirtualWorkspaceConfig {

View File

@ -5,6 +5,8 @@ import {
ProjectFileMap, ProjectFileMap,
ProjectGraph, ProjectGraph,
} from '../../../../config/project-graph'; } from '../../../../config/project-graph';
import { join, relative } from 'path';
import { workspaceRoot } from '../../../../utils/workspace-root';
export type ExplicitDependency = { export type ExplicitDependency = {
sourceProjectName: string; sourceProjectName: string;
@ -18,10 +20,7 @@ export function buildExplicitTypeScriptDependencies(
filesToProcess: ProjectFileMap filesToProcess: ProjectFileMap
): ExplicitDependency[] { ): ExplicitDependency[] {
let results: ExplicitDependency[]; let results: ExplicitDependency[];
if ( if (process.env.NX_NATIVE_TS_DEPS !== 'false') {
process.env.NX_NATIVE_TS_DEPS &&
process.env.NX_NATIVE_TS_DEPS !== 'false'
) {
results = buildExplicitTypeScriptDependenciesWithSwc(filesToProcess, graph); results = buildExplicitTypeScriptDependenciesWithSwc(filesToProcess, graph);
} else { } else {
results = buildExplicitTypeScriptDependenciesWithTs(filesToProcess, graph); results = buildExplicitTypeScriptDependenciesWithTs(filesToProcess, graph);
@ -109,7 +108,7 @@ function buildExplicitTypeScriptDependenciesWithSwc(
filesToProcess[project] ??= []; filesToProcess[project] ??= [];
for (const { file } of fileData) { for (const { file } of fileData) {
if (moduleExtensions.some((ext) => file.endsWith(ext))) { if (moduleExtensions.some((ext) => file.endsWith(ext))) {
filesToProcess[project].push(file); filesToProcess[project].push(join(workspaceRoot, file));
} }
} }
} }
@ -127,7 +126,7 @@ function buildExplicitTypeScriptDependenciesWithSwc(
for (const importExpr of staticImportExpressions) { for (const importExpr of staticImportExpressions) {
const dependency = convertImportToDependency( const dependency = convertImportToDependency(
importExpr, importExpr,
file, relative(workspaceRoot, file),
sourceProject, sourceProject,
DependencyType.static, DependencyType.static,
targetProjectLocator targetProjectLocator
@ -143,7 +142,7 @@ function buildExplicitTypeScriptDependenciesWithSwc(
for (const importExpr of dynamicImportExpressions) { for (const importExpr of dynamicImportExpressions) {
const dependency = convertImportToDependency( const dependency = convertImportToDependency(
importExpr, importExpr,
file, relative(workspaceRoot, file),
sourceProject, sourceProject,
DependencyType.dynamic, DependencyType.dynamic,
targetProjectLocator targetProjectLocator