Skip to content

Commit

Permalink
fix(core): Update bundlers to not typecheck if using new TS solution …
Browse files Browse the repository at this point in the history
…setup (#29227)

If we are using the new TS setup we should opt out of doing type
checking during build since we already have a typecheck target and it
may lead to doing type checking twice.
  • Loading branch information
ndcunningham authored Dec 9, 2024
1 parent 04151ca commit 89ab887
Show file tree
Hide file tree
Showing 13 changed files with 47 additions and 14 deletions.
7 changes: 5 additions & 2 deletions packages/esbuild/src/executors/esbuild/esbuild.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ export async function* esbuildExecutor(
name: 'nx-watch-plugin',
setup(build: esbuild.PluginBuild) {
build.onEnd(async (result: esbuild.BuildResult) => {
if (!options.skipTypeCheck) {
if (
!options.skipTypeCheck &&
!options.isTsSolutionSetup
) {
const { errors } = await runTypeCheck(
options,
context
Expand Down Expand Up @@ -180,7 +183,7 @@ export async function* esbuildExecutor(
);
} else {
// Run type-checks first and bail if they don't pass.
if (!options.skipTypeCheck) {
if (!options.skipTypeCheck && !options.isTsSolutionSetup) {
const { errors } = await runTypeCheck(options, context);
if (errors.length > 0) {
yield { success: false };
Expand Down
4 changes: 4 additions & 0 deletions packages/esbuild/src/executors/esbuild/lib/normalize.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe('normalizeOptions', () => {
singleEntry: true,
external: [],
thirdParty: false,
isTsSolutionSetup: false,
});
});

Expand Down Expand Up @@ -93,6 +94,7 @@ describe('normalizeOptions', () => {
singleEntry: false,
external: [],
thirdParty: false,
isTsSolutionSetup: false,
});
});

Expand All @@ -119,6 +121,7 @@ describe('normalizeOptions', () => {
singleEntry: true,
external: [],
thirdParty: false,
isTsSolutionSetup: false,
});
});

Expand Down Expand Up @@ -162,6 +165,7 @@ describe('normalizeOptions', () => {
singleEntry: true,
external: [],
thirdParty: false,
isTsSolutionSetup: false,
});
});

Expand Down
2 changes: 2 additions & 0 deletions packages/esbuild/src/executors/esbuild/lib/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export function normalizeOptions(
userDefinedBuildOptions,
external: options.external ?? [],
singleEntry: false,
isTsSolutionSetup,
// Use the `main` file name as the output file name.
// This is needed for `@nx/js:node` to know the main file to execute.
// NOTE: The .js default extension may be replaced later in getOutfile() call.
Expand All @@ -126,6 +127,7 @@ export function normalizeOptions(
userDefinedBuildOptions,
external: options.external ?? [],
singleEntry: true,
isTsSolutionSetup,
outputFileName:
// NOTE: The .js default extension may be replaced later in getOutfile() call.
options.outputFileName ?? `${path.parse(options.main).name}.js`,
Expand Down
1 change: 1 addition & 0 deletions packages/esbuild/src/executors/esbuild/schema.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ export interface NormalizedEsBuildExecutorOptions
singleEntry: boolean;
external: string[];
userDefinedBuildOptions: esbuild.BuildOptions | undefined;
isTsSolutionSetup?: boolean;
}
2 changes: 1 addition & 1 deletion packages/js/src/executors/swc/swc.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function normalizeOptions(

const outputPath = join(root, options.outputPath);

if (options.skipTypeCheck == null) {
if (options.skipTypeCheck == null && !isTsSolutionSetup) {
options.skipTypeCheck = false;
}

Expand Down
7 changes: 5 additions & 2 deletions packages/js/src/generators/library/library.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,10 @@ async function configureProject(
projectConfiguration.targets.build.options.format = ['cjs'];
}

if (options.bundler === 'swc' && options.skipTypeCheck) {
if (
options.bundler === 'swc' &&
(options.skipTypeCheck || options.isUsingTsSolutionConfig)
) {
projectConfiguration.targets.build.options.skipTypeCheck = true;
}

Expand Down Expand Up @@ -783,7 +786,7 @@ async function normalizeOptions(

if (
(options.bundler === 'swc' || options.bundler === 'rollup') &&
options.skipTypeCheck == null
(options.skipTypeCheck == null || !isUsingTsSolutionConfig)
) {
options.skipTypeCheck = false;
}
Expand Down
7 changes: 5 additions & 2 deletions packages/js/src/utils/package-manager-workspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ export function isWorkspacesEnabled(
}

// yarn and npm both use the same 'workspaces' property in package.json
const packageJson = readJson<PackageJson>(tree, 'package.json');
return !!packageJson?.workspaces;
if (tree.exists('package.json')) {
const packageJson = readJson<PackageJson>(tree, 'package.json');
return !!packageJson?.workspaces;
}
return false;
}

export function getProjectPackageManagerWorkspaceStateWarningTask(
Expand Down
7 changes: 5 additions & 2 deletions packages/js/src/utils/swc/compile-swc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export async function compileSwc(
logger.log(swcCmdLog.replace(/\n/, ''));
const isCompileSuccess = swcCmdLog.includes('Successfully compiled');

if (normalizedOptions.skipTypeCheck) {
if (normalizedOptions.skipTypeCheck || normalizedOptions.isTsSolutionSetup) {
await postCompilationCallback();
return { success: isCompileSuccess };
}
Expand Down Expand Up @@ -159,7 +159,10 @@ export async function* compileSwcWatch(
initialPostCompile = false;
}

if (normalizedOptions.skipTypeCheck) {
if (
normalizedOptions.skipTypeCheck ||
normalizedOptions.isTsSolutionSetup
) {
next(getResult(swcStatus));
return;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/rollup/src/executors/rollup/lib/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { resolve } from 'path';
import { ExecutorContext } from '@nx/devkit';

import type { RollupExecutorOptions } from '../schema';
import { isUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup';

export interface NormalizedRollupExecutorOptions extends RollupExecutorOptions {
projectRoot: string;
Expand All @@ -13,14 +14,15 @@ export function normalizeRollupExecutorOptions(
context: ExecutorContext
): NormalizedRollupExecutorOptions {
const { root } = context;
const skipTypeCheck = isUsingTsSolutionSetup() ? true : options.skipTypeCheck;
return {
...options,
rollupConfig: []
.concat(options.rollupConfig)
.filter(Boolean)
.map((p) => normalizePluginPath(p, root)),
projectRoot: context.projectGraph.nodes[context.projectName].data.root,
skipTypeCheck: options.skipTypeCheck || false,
skipTypeCheck: skipTypeCheck || false,
};
}

Expand Down
7 changes: 7 additions & 0 deletions packages/rollup/src/plugins/with-nx/normalize-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
RollupWithNxPluginOptions,
} from './with-nx-options';
import { createEntryPoints } from '@nx/js';
import { isUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup';

export function normalizeOptions(
projectRoot: string,
Expand All @@ -16,6 +17,12 @@ export function normalizeOptions(
if (global.NX_GRAPH_CREATION)
return options as NormalizedRollupWithNxPluginOptions;
normalizeRelativePaths(projectRoot, options);

// New TS Solution already has a typecheck target
if (isUsingTsSolutionSetup()) {
options.skipTypeCheck = true;
}

return {
...options,
additionalEntryPoints: createEntryPoints(
Expand Down
4 changes: 3 additions & 1 deletion packages/rspack/src/plugins/utils/apply-base-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { NxTsconfigPathsRspackPlugin } from './plugins/nx-tsconfig-paths-rspack-
import { getTerserEcmaVersion } from './get-terser-ecma-version';
import nodeExternals = require('webpack-node-externals');
import { NormalizedNxAppRspackPluginOptions } from './models';
import { isUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup';

const IGNORED_RSPACK_WARNINGS = [
/The comment file/i,
Expand Down Expand Up @@ -226,7 +227,8 @@ function applyNxDependentConfig(

plugins.push(new NxTsconfigPathsRspackPlugin({ ...options, tsConfig }));

if (!options?.skipTypeChecking) {
// New TS Solution already has a typecheck target
if (!options?.skipTypeChecking && !isUsingTsSolutionSetup()) {
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
plugins.push(
new ForkTsCheckerWebpackPlugin({
Expand Down
5 changes: 3 additions & 2 deletions packages/vite/src/executors/build/build.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
validateTypes,
} from '../../utils/executor-utils';
import { type Plugin } from 'vite';
import { isUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup';

export async function* viteBuildExecutor(
options: Record<string, any> & ViteBuildExecutorOptions,
Expand Down Expand Up @@ -85,8 +86,8 @@ export async function* viteBuildExecutor(
...otherOptions,
}
);

if (!options.skipTypeCheck) {
// New TS Solution already has a typecheck target
if (!options.skipTypeCheck && !isUsingTsSolutionSetup()) {
await validateTypes({
workspaceRoot: context.root,
tsconfig: tsConfigForBuild,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { createLoaderFromCompiler } from './compiler-loaders';
import { NormalizedNxAppWebpackPluginOptions } from '../nx-app-webpack-plugin-options';
import TerserPlugin = require('terser-webpack-plugin');
import nodeExternals = require('webpack-node-externals');
import { isUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup';

const IGNORED_WEBPACK_WARNINGS = [
/The comment file/i,
Expand Down Expand Up @@ -232,7 +233,8 @@ function applyNxDependentConfig(

plugins.push(new NxTsconfigPathsWebpackPlugin({ ...options, tsConfig }));

if (!options?.skipTypeChecking) {
// New TS Solution already has a typecheck target
if (!options?.skipTypeChecking && !isUsingTsSolutionSetup()) {
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
plugins.push(
new ForkTsCheckerWebpackPlugin({
Expand Down

0 comments on commit 89ab887

Please sign in to comment.