Skip to content

Commit

Permalink
fix(common): update TS module resolution flow
Browse files Browse the repository at this point in the history
  • Loading branch information
arturovt committed Feb 4, 2024
1 parent 3b194f1 commit 0de7266
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 116 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import type { Configuration } from 'webpack';

import { WebpackEsmPlugin } from 'webpack-esm-plugin';

export default async (cfg: Configuration) => {
const { default: configFromEsm } = await import('./custom-webpack.config.js');

// This is used to ensure we fixed the following issue:
// https://github.com/just-jeb/angular-builders/issues/1213
cfg.plugins!.push(new WebpackEsmPlugin());

// Do some stuff with config and configFromEsm
return { ...cfg, ...configFromEsm };
};
3 changes: 2 additions & 1 deletion examples/custom-webpack/sanity-app-esm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"karma-jasmine": "5.1.0",
"karma-jasmine-html-reporter": "2.1.0",
"puppeteer": "21.10.0",
"typescript": "5.3.3"
"typescript": "5.3.3",
"webpack-esm-plugin": "file:./webpack-esm-plugin"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"name": "webpack-esm-plugin",
"version": "0.0.1",
"module": "./webpack-esm-plugin.mjs",
"typings": "./webpack-esm-plugin.d.ts",
"exports": {
"./package.json": {
"default": "./package.json"
},
".": {
"types": "./webpack-esm-plugin.d.ts",
"node": "./webpack-esm-plugin.mjs",
"default": "./webpack-esm-plugin.mjs"
}
},
"sideEffects": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import * as webpack from 'webpack';

export declare class WebpackEsmPlugin {
apply(compiler: webpack.Compiler): void;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class WebpackEsmPlugin {
apply(compiler) {
console.error('hello from the WebpackEsmPlugin');
}
}

export { WebpackEsmPlugin };
82 changes: 9 additions & 73 deletions packages/common/src/load-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,56 +2,7 @@ import * as path from 'node:path';
import * as url from 'node:url';
import type { logging } from '@angular-devkit/core';

const _tsNodeRegister = (() => {
let lastTsConfig: string | undefined;
return (tsConfig: string, logger: logging.LoggerApi) => {
// Check if the function was previously called with the same tsconfig
if (lastTsConfig && lastTsConfig !== tsConfig) {
logger.warn(`Trying to register ts-node again with a different tsconfig - skipping the registration.
tsconfig 1: ${lastTsConfig}
tsconfig 2: ${tsConfig}`);
}

if (lastTsConfig) {
return;
}

lastTsConfig = tsConfig;

loadTsNode().register({
project: tsConfig,
compilerOptions: {
module: 'CommonJS',
types: [
'node', // NOTE: `node` is added because users scripts can also use pure node's packages as webpack or others
],
},
});

const tsConfigPaths = loadTsConfigPaths();
const result = tsConfigPaths.loadConfig(tsConfig);
// The `loadConfig` returns a `ConfigLoaderResult` which must be guarded with
// the `resultType` check.
if (result.resultType === 'success') {
const { absoluteBaseUrl: baseUrl, paths } = result;
if (baseUrl && paths) {
tsConfigPaths.register({ baseUrl, paths });
}
}
};
})();

/**
* check for TS node registration
* @param file: file name or file directory are allowed
* @todo tsNodeRegistration: require ts-node if file extension is TypeScript
*/
function tsNodeRegister(file: string = '', tsConfig: string, logger: logging.LoggerApi) {
if (file?.endsWith('.ts')) {
// Register TS compiler lazily
_tsNodeRegister(tsConfig, logger);
}
}
import { registerTsProject } from './register-ts-project';

/**
* This uses a dynamic import to load a module which may be ESM.
Expand All @@ -72,22 +23,20 @@ function loadEsmModule<T>(modulePath: string | URL): Promise<T> {
/**
* Loads CJS and ESM modules based on extension
*/
export async function loadModule<T>(
modulePath: string,
tsConfig: string,
logger: logging.LoggerApi
): Promise<T> {
tsNodeRegister(modulePath, tsConfig, logger);

export async function loadModule<T>(modulePath: string, tsConfig: string): Promise<T> {
switch (path.extname(modulePath)) {
case '.mjs':
// Load the ESM configuration file using the TypeScript dynamic import workaround.
// Once TypeScript provides support for keeping the dynamic import this workaround can be
// changed to a direct dynamic import.
return (await loadEsmModule<{ default: T }>(url.pathToFileURL(modulePath))).default;

case '.cjs':
return require(modulePath);

case '.ts':
const unregisterTsProject = registerTsProject(tsConfig);

try {
// If it's a TS file then there are 2 cases for exporing an object.
// The first one is `export blah`, transpiled into `module.exports = { blah} `.
Expand All @@ -101,7 +50,10 @@ export async function loadModule<T>(
return (await loadEsmModule<{ default: T }>(url.pathToFileURL(modulePath))).default;
}
throw e;
} finally {
unregisterTsProject();
}

//.js
default:
// The file could be either CommonJS or ESM.
Expand All @@ -120,19 +72,3 @@ export async function loadModule<T>(
}
}
}

/**
* Loads `ts-node` lazily. Moved to a separate function to declare
* a return type, more readable than an inline variant.
*/
function loadTsNode(): typeof import('ts-node') {
return require('ts-node');
}

/**
* Loads `tsconfig-paths` lazily. Moved to a separate function to declare
* a return type, more readable than an inline variant.
*/
function loadTsConfigPaths(): typeof import('tsconfig-paths') {
return require('tsconfig-paths');
}
87 changes: 87 additions & 0 deletions packages/common/src/register-ts-project.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import * as path from 'node:path';
import type { CompilerOptions } from 'typescript';

let ts: typeof import('typescript');
let isTsEsmLoaderRegistered = false;

export function registerTsProject(tsConfig: string) {
const cleanupFunctions = [registerTsConfigPaths(tsConfig), registerTsNodeService(tsConfig)];

// Add ESM support for `.ts` files.
// NOTE: There is no cleanup function for this, as it's not possible to unregister the loader.
// Based on limited testing, it doesn't seem to matter if we register it multiple times, but just in
// case let's keep a flag to prevent it.
if (!isTsEsmLoaderRegistered) {
const module = require('node:module');
if (module.register && packageIsInstalled('ts-node/esm')) {
const url = require('node:url');
module.register(url.pathToFileURL(require.resolve('ts-node/esm')));
}
isTsEsmLoaderRegistered = true;
}

return () => {
cleanupFunctions.forEach(fn => fn());
};
}

function registerTsNodeService(tsConfig: string): VoidFunction {
const { register } = require('ts-node') as typeof import('ts-node');

const service = register({
project: tsConfig,
compilerOptions: {
module: 'CommonJS',
types: [
'node', // NOTE: `node` is added because users scripts can also use pure node's packages as webpack or others
],
},
});

return () => {
service.enabled(false);
};
}

function registerTsConfigPaths(tsConfig: string): VoidFunction {
const tsConfigPaths = require('tsconfig-paths') as typeof import('tsconfig-paths');
const result = tsConfigPaths.loadConfig(tsConfig);
if (result.resultType === 'success') {
const { absoluteBaseUrl: baseUrl, paths } = result;
if (baseUrl && paths) {
// Returns a function to undo paths registration.
return tsConfigPaths.register({ baseUrl, paths });
}
}

// We cannot return anything here if paths failed to be registered.
// Additionally, I don't think we should perform any logging in this
// context, considering that this is internal information not exposed
// to the end user
return () => {};
}

function packageIsInstalled(m: string): boolean {
try {
require.resolve(m);
return true;
} catch {
return false;
}
}

function readCompilerOptions(tsConfig: string): CompilerOptions {
ts ??= require('typescript');

const jsonContent = ts.readConfigFile(tsConfig, ts.sys.readFile);
const { options } = ts.parseJsonConfigFileContent(
jsonContent.config,
ts.sys,
path.dirname(tsConfig)
);

// This property is returned in compiler options for some reason, but not part of the typings.
// ts-node fails on unknown props, so we have to remove it.
delete options.configFilePath;
return options;
}
8 changes: 2 additions & 6 deletions packages/custom-esbuild/src/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,10 @@ export function buildCustomEsbuildApplication(
const tsConfig = path.join(workspaceRoot, options.tsConfig);

return defer(async () => {
const codePlugins = await loadPlugins(options.plugins, workspaceRoot, tsConfig, context.logger);
const codePlugins = await loadPlugins(options.plugins, workspaceRoot, tsConfig);

const indexHtmlTransformer = options.indexHtmlTransformer
? await loadModule(
path.join(workspaceRoot, options.indexHtmlTransformer),
tsConfig,
context.logger
)
? await loadModule(path.join(workspaceRoot, options.indexHtmlTransformer), tsConfig)
: undefined;

return { codePlugins, indexHtmlTransformer } as ApplicationBuilderExtensions;
Expand Down
19 changes: 3 additions & 16 deletions packages/custom-esbuild/src/dev-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,14 @@ export function executeCustomDevServerBuilder(
const middleware = await Promise.all(
(options.middlewares || []).map(middlewarePath =>
// https://github.com/angular/angular-cli/pull/26212/files#diff-a99020cbdb97d20b2bc686bcb64b31942107d56db06fd880171b0a86f7859e6eR52
loadModule<Connect.NextHandleFunction>(
path.join(workspaceRoot, middlewarePath),
tsConfig,
context.logger
)
loadModule<Connect.NextHandleFunction>(path.join(workspaceRoot, middlewarePath), tsConfig)
)
);

const buildPlugins = await loadPlugins(
buildOptions.plugins,
workspaceRoot,
tsConfig,
context.logger
);
const buildPlugins = await loadPlugins(buildOptions.plugins, workspaceRoot, tsConfig);

const indexHtmlTransformer: IndexHtmlTransform = buildOptions.indexHtmlTransformer
? await loadModule(
path.join(workspaceRoot, buildOptions.indexHtmlTransformer),
tsConfig,
context.logger
)
? await loadModule(path.join(workspaceRoot, buildOptions.indexHtmlTransformer), tsConfig)
: undefined;

patchBuilderContext(context, buildTarget);
Expand Down
6 changes: 2 additions & 4 deletions packages/custom-esbuild/src/load-plugins.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import * as path from 'node:path';
import type { Plugin } from 'esbuild';
import type { logging } from '@angular-devkit/core';
import { loadModule } from '@angular-builders/common';

export async function loadPlugins(
paths: string[] | undefined,
workspaceRoot: string,
tsConfig: string,
logger: logging.LoggerApi
tsConfig: string
): Promise<Plugin[]> {
const plugins = await Promise.all(
(paths || []).map(pluginPath =>
loadModule<Plugin | Plugin[]>(path.join(workspaceRoot, pluginPath), tsConfig, logger)
loadModule<Plugin | Plugin[]>(path.join(workspaceRoot, pluginPath), tsConfig)
)
);

Expand Down
3 changes: 1 addition & 2 deletions packages/custom-webpack/src/custom-webpack-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ export class CustomWebpackBuilder {
);
const configOrFactoryOrPromise = await loadModule<CustomWebpackConfig>(
webpackConfigPath,
tsConfig,
logger
tsConfig
);

if (typeof configOrFactoryOrPromise === 'function') {
Expand Down
11 changes: 6 additions & 5 deletions packages/custom-webpack/src/transform-factories.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
jest.mock('ts-node', () => ({
register: jest.fn(),
register: jest.fn().mockReturnValue({
enabled: jest.fn(),
}),
}));
jest.mock('tsconfig-paths', () => ({
loadConfig: jest.fn().mockReturnValue({}),
loadConfig: jest.fn().mockReturnValue({
register: jest.fn().mockReturnValue(() => {}),
}),
}));
import { getTransforms } from './transform-factories';

Expand Down Expand Up @@ -32,7 +36,6 @@ describe('getTransforms', () => {
transforms.webpackConfiguration({});

expect(tsNode.register).toHaveBeenCalledTimes(1);
expect(logger.warn).not.toHaveBeenCalled();

const transforms2 = getTransforms(
{
Expand All @@ -45,7 +48,5 @@ describe('getTransforms', () => {
{ workspaceRoot: './test', logger } as any
);
transforms2.webpackConfiguration({});

expect(logger.warn).toHaveBeenCalledTimes(1);
});
});
9 changes: 2 additions & 7 deletions packages/custom-webpack/src/transform-factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,14 @@ export const customWebpackConfigTransformFactory: (
export const indexHtmlTransformFactory: (
options: CustomWebpackSchema,
context: BuilderContext
) => IndexHtmlTransform = (options, { workspaceRoot, target, logger }) => {
) => IndexHtmlTransform = (options, { workspaceRoot, target }) => {
if (!options.indexTransform) return null;

const transformPath = path.join(getSystemPath(normalize(workspaceRoot)), options.indexTransform);
const tsConfig = path.join(getSystemPath(normalize(workspaceRoot)), options.tsConfig);

return async (indexHtml: string) => {
const transform = await loadModule<IndexHtmlTransformWithOptions>(
transformPath,
tsConfig,
logger
);

const transform = await loadModule<IndexHtmlTransformWithOptions>(transformPath, tsConfig);
return transform(target, indexHtml);
};
};
Expand Down
Loading

0 comments on commit 0de7266

Please sign in to comment.