From 40da73f18303cb4e3738cb9e28b0d7862af46acb Mon Sep 17 00:00:00 2001 From: Colum Ferry Date: Thu, 20 Jun 2024 12:32:15 +0100 Subject: [PATCH] chore(remix): add unit tests --- .../convert-to-inferred.spec.ts | 440 ++++++++++++++++++ .../lib/build-post-target-transformer.spec.ts | 137 ++++++ .../lib/build-post-target-transformer.ts | 87 +--- .../lib/serve-post-target-transformer.ts | 10 +- .../convert-to-inferred/lib/utils.ts | 39 -- 5 files changed, 589 insertions(+), 124 deletions(-) diff --git a/packages/remix/src/generators/convert-to-inferred/convert-to-inferred.spec.ts b/packages/remix/src/generators/convert-to-inferred/convert-to-inferred.spec.ts index e69de29bb2d1d..aa3ff36130c85 100644 --- a/packages/remix/src/generators/convert-to-inferred/convert-to-inferred.spec.ts +++ b/packages/remix/src/generators/convert-to-inferred/convert-to-inferred.spec.ts @@ -0,0 +1,440 @@ +import { + type ProjectGraph, + type Tree, + type ProjectConfiguration, + joinPathFragments, + writeJson, + addProjectConfiguration, + readProjectConfiguration, + readNxJson, + type ExpandedPluginConfiguration, + updateNxJson, +} from '@nx/devkit'; +import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; +import { TempFs } from '@nx/devkit/internal-testing-utils'; +import { getRelativeProjectJsonSchemaPath } from 'nx/src/generators/utils/project-configuration'; +import { join } from 'path'; +import { convertToInferred } from './convert-to-inferred'; + +let fs: TempFs; +let projectGraph: ProjectGraph; + +jest.mock('@nx/devkit', () => ({ + ...jest.requireActual('@nx/devkit'), + createProjectGraphAsync: jest + .fn() + .mockImplementation(() => Promise.resolve(projectGraph)), + updateProjectConfiguration: jest + .fn() + .mockImplementation((tree, projectName, projectConfiguration) => { + function handleEmptyTargets( + projectName: string, + projectConfiguration: ProjectConfiguration + ): void { + if ( + projectConfiguration.targets && + !Object.keys(projectConfiguration.targets).length + ) { + // Re-order `targets` to appear after the `// target` comment. + delete projectConfiguration.targets; + projectConfiguration[ + '// targets' + ] = `to see all targets run: nx show project ${projectName} --web`; + projectConfiguration.targets = {}; + } else { + delete projectConfiguration['// targets']; + } + } + + const projectConfigFile = joinPathFragments( + projectConfiguration.root, + 'project.json' + ); + + if (!tree.exists(projectConfigFile)) { + throw new Error( + `Cannot update Project ${projectName} at ${projectConfiguration.root}. It either doesn't exist yet, or may not use project.json for configuration. Use \`addProjectConfiguration()\` instead if you want to create a new project.` + ); + } + handleEmptyTargets(projectName, projectConfiguration); + writeJson(tree, projectConfigFile, { + name: projectConfiguration.name ?? projectName, + $schema: getRelativeProjectJsonSchemaPath(tree, projectConfiguration), + ...projectConfiguration, + root: undefined, + }); + projectGraph.nodes[projectName].data = projectConfiguration; + }), +})); + +function addProject(tree: Tree, name: string, project: ProjectConfiguration) { + addProjectConfiguration(tree, name, project); + projectGraph.nodes[name] = { + name, + type: project.projectType === 'application' ? 'app' : 'lib', + data: { + projectType: project.projectType, + root: project.root, + targets: project.targets, + }, + }; +} + +interface TestProjectOptions { + appName: string; + appRoot: string; + outputPath: string; + buildTargetName: string; + serveTargetName: string; +} + +const defaultTestProjectOptions: TestProjectOptions = { + appName: 'app1', + appRoot: 'apps/app1', + outputPath: 'dist/apps/app1', + buildTargetName: 'build', + serveTargetName: 'serve', +}; + +function writeRemixConfig(tree: Tree, projectRoot: string) { + const remixConfig = { + ignoredRouteFiles: ['**/.*'], + }; + const remixConfigContents = `const config = ${JSON.stringify(remixConfig)}; +export default config;`; + + tree.write(`${projectRoot}/remix.config.js`, remixConfigContents); + fs.createFileSync(`${projectRoot}/remix.config.js`, remixConfigContents); + tree.write(`${projectRoot}/package.json`, `{"type":"module"}`); + fs.createFileSync(`${projectRoot}/package.json`, `{"type":"module"}`); + jest.doMock( + join(fs.tempDir, projectRoot, 'remix.config.js'), + () => remixConfig, + { virtual: true } + ); +} + +function createTestProject( + tree: Tree, + opts: Partial = defaultTestProjectOptions, + extraTargetOptions: any = {}, + extraConfigurations: any = {} +) { + let projectOpts = { ...defaultTestProjectOptions, ...opts }; + const project: ProjectConfiguration = { + name: projectOpts.appName, + root: projectOpts.appRoot, + projectType: 'application', + targets: { + [projectOpts.buildTargetName]: { + executor: '@nx/remix:build', + options: { + outputPath: projectOpts.outputPath, + ...extraTargetOptions, + }, + configurations: { + ...extraConfigurations, + }, + }, + [projectOpts.serveTargetName]: { + executor: '@nx/remix:serve', + options: { + port: 4200, + ...extraTargetOptions, + }, + configurations: { + ...extraConfigurations, + }, + }, + }, + }; + + writeRemixConfig(tree, project.root); + + addProject(tree, project.name, project); + fs.createFileSync( + `${projectOpts.appRoot}/project.json`, + JSON.stringify(project) + ); + return project; +} + +describe('Remix - Convert To Inferred', () => { + let tree: Tree; + beforeEach(() => { + fs = new TempFs('remix'); + tree = createTreeWithEmptyWorkspace(); + tree.root = fs.tempDir; + + projectGraph = { + nodes: {}, + dependencies: {}, + externalNodes: {}, + }; + }); + + afterEach(() => { + fs.cleanup(); + jest.resetModules(); + }); + + describe('--project', () => { + it('should correctly migrate a single project', async () => { + // ARRANGE + const project = createTestProject(tree); + const project2 = createTestProject(tree, { + appRoot: 'apps/project2', + appName: 'project2', + }); + + const project2Targets = project2.targets; + + // ACT + await convertToInferred(tree, { + project: project.name, + skipFormat: true, + }); + + // ASSERT + const updatedProject = readProjectConfiguration(tree, project.name); + expect(updatedProject.targets).toMatchInlineSnapshot(` + { + "serve": { + "options": { + "env": { + "PORT": "4200", + }, + }, + }, + } + `); + + const updatedProject2 = readProjectConfiguration(tree, project2.name); + expect(updatedProject2.targets).toStrictEqual(project2Targets); + + const nxJsonPlugins = readNxJson(tree).plugins; + const remixPlugin = nxJsonPlugins.find( + (plugin): plugin is ExpandedPluginConfiguration => + typeof plugin !== 'string' && + plugin.plugin === '@nx/remix/plugin' && + plugin.include?.length === 1 + ); + expect(remixPlugin).toBeTruthy(); + expect(remixPlugin.include).toEqual([`${project.root}/**/*`]); + }); + + it('should add a new plugin registration when the target name differs', async () => { + // ARRANGE + const project = createTestProject(tree); + const project2 = createTestProject(tree, { + appRoot: 'apps/project2', + appName: 'project2', + }); + + const project2Targets = project2.targets; + + const nxJson = readNxJson(tree); + nxJson.plugins ??= []; + nxJson.plugins.push({ + plugin: '@nx/remix/plugin', + options: { + buildTargetName: 'build', + devTargetName: defaultTestProjectOptions.serveTargetName, + startTargetName: 'start', + typecheckTargetName: 'typecheck', + staticServeTargetName: 'static-serve', + }, + }); + updateNxJson(tree, nxJson); + + // ACT + await convertToInferred(tree, { + project: project.name, + skipFormat: true, + }); + + // ASSERT + const updatedProject = readProjectConfiguration(tree, project.name); + expect(updatedProject.targets).toMatchInlineSnapshot(` + { + "serve": { + "options": { + "env": { + "PORT": "4200", + }, + }, + }, + } + `); + + const updatedProject2 = readProjectConfiguration(tree, project2.name); + expect(updatedProject2.targets).toStrictEqual(project2Targets); + + const nxJsonPlugins = readNxJson(tree).plugins; + const remixPluginRegistrations = nxJsonPlugins.filter( + (plugin): plugin is ExpandedPluginConfiguration => + typeof plugin !== 'string' && plugin.plugin === '@nx/remix/plugin' + ); + expect(remixPluginRegistrations.length).toBe(2); + expect(remixPluginRegistrations[1].include).toMatchInlineSnapshot(` + [ + "apps/app1/**/*", + ] + `); + }); + + it('should merge target defaults', async () => { + // ARRANGE + const project = createTestProject(tree); + const project2 = createTestProject(tree, { + appRoot: 'apps/project2', + appName: 'project2', + }); + + const project2Targets = project2.targets; + + const nxJson = readNxJson(tree); + nxJson.targetDefaults ??= {}; + nxJson.targetDefaults['@nx/remix:build'] = { + options: { + sourcemap: true, + }, + }; + updateNxJson(tree, nxJson); + + // ACT + await convertToInferred(tree, { + project: project.name, + skipFormat: true, + }); + + // ASSERT + const updatedProject = readProjectConfiguration(tree, project.name); + expect(updatedProject.targets).toMatchInlineSnapshot(` + { + "build": { + "options": { + "sourcemap": true, + }, + }, + "serve": { + "options": { + "env": { + "PORT": "4200", + }, + }, + }, + } + `); + + const updatedProject2 = readProjectConfiguration(tree, project2.name); + expect(updatedProject2.targets).toStrictEqual(project2Targets); + }); + + it('should manage configurations correctly', async () => { + // ARRANGE + const project = createTestProject(tree, undefined, undefined, { + dev: { + outputPath: 'apps/dev/app1', + }, + }); + const project2 = createTestProject(tree, { + appRoot: 'apps/project2', + appName: 'project2', + }); + + const project2Targets = project2.targets; + // ACT + await convertToInferred(tree, { + project: project.name, + skipFormat: true, + }); + + // ASSERT + const updatedProject = readProjectConfiguration(tree, project.name); + expect(updatedProject.targets).toMatchInlineSnapshot(` + { + "build": { + "configurations": { + "dev": {}, + }, + }, + "serve": { + "configurations": { + "dev": { + "outputPath": "apps/dev/app1", + }, + }, + "options": { + "env": { + "PORT": "4200", + }, + }, + }, + } + `); + + const updatedProject2 = readProjectConfiguration(tree, project2.name); + expect(updatedProject2.targets).toStrictEqual(project2Targets); + + const remixConfigContents = tree.read( + `${project.root}/remix.config.js`, + 'utf-8' + ); + expect(remixConfigContents).toMatchInlineSnapshot(` + "const config = {"ignoredRouteFiles":["**/.*"]}; + export default config;" + `); + }); + }); + + describe('all projects', () => { + it('should correctly migrate all projects', async () => { + // ARRANGE + const project = createTestProject(tree); + const project2 = createTestProject(tree, { + appRoot: 'apps/project2', + appName: 'project2', + }); + // ACT + await convertToInferred(tree, { + skipFormat: true, + }); + + // ASSERT + const updatedProject = readProjectConfiguration(tree, project.name); + expect(updatedProject.targets).toMatchInlineSnapshot(` + { + "serve": { + "options": { + "env": { + "PORT": "4200", + }, + }, + }, + } + `); + + const updatedProject2 = readProjectConfiguration(tree, project2.name); + expect(updatedProject2.targets).toMatchInlineSnapshot(` + { + "serve": { + "options": { + "env": { + "PORT": "4200", + }, + }, + }, + } + `); + + const nxJsonPlugins = readNxJson(tree).plugins; + const remixPlugin = nxJsonPlugins.find( + (plugin): plugin is ExpandedPluginConfiguration => + typeof plugin !== 'string' && plugin.plugin === '@nx/remix/plugin' + ); + expect(remixPlugin).toBeTruthy(); + expect(remixPlugin.include).toBeUndefined(); + }); + }); +}); diff --git a/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.spec.ts b/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.spec.ts index e69de29bb2d1d..f94bf3b78c4d3 100644 --- a/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.spec.ts +++ b/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.spec.ts @@ -0,0 +1,137 @@ +import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; +import { AggregatedLog } from '@nx/devkit/src/generators/plugin-migrations/aggregate-log-util'; +import { buildPostTargetTransformer } from './build-post-target-transformer'; + +describe('buildPostTargetTransformer', () => { + it('should migrate outputPath correctly', () => { + // ARRANGE + const tree = createTreeWithEmptyWorkspace(); + + const targetConfiguration = { + options: { + outputPath: 'dist/apps/myapp', + }, + }; + + const inferredTargetConfiguration = {}; + + const migrationLogs = new AggregatedLog(); + + tree.write('apps/myapp/remix.config.js', remixConfig); + tree.write('apps/myapp/package.json', `{"type": "module"}`); + + // ACT + const target = buildPostTargetTransformer(migrationLogs)( + targetConfiguration, + tree, + { projectName: 'myapp', root: 'apps/myapp' }, + inferredTargetConfiguration + ); + + // ASSERT + const configFile = tree.read('apps/myapp/remix.config.js', 'utf-8'); + expect(configFile).toMatchInlineSnapshot(` + "import { createWatchPaths } from '@nx/remix'; + import { dirname } from 'path'; + import { fileURLToPath } from 'url'; + + const __dirname = dirname(fileURLToPath(import.meta.url)); + + /** + * @type {import('@remix-run/dev').AppConfig} + */ + export default { + ignoredRouteFiles: ['**/.*'], + // appDirectory: "app", + // assetsBuildDirectory: "public/build", + // serverBuildPath: "build/index.js", + // publicPath: "/build/", + watchPaths: () => createWatchPaths(__dirname), + };" + `); + expect(target).toMatchInlineSnapshot(` + { + "options": {}, + } + `); + }); + + it('should handle configurations correctly', () => { + // ARRANGE + const tree = createTreeWithEmptyWorkspace(); + + const targetConfiguration = { + options: { + outputPath: 'dist/apps/myapp', + }, + configurations: { + dev: { + outputPath: 'dist/dev/apps/myapp', + }, + }, + }; + + const inferredTargetConfiguration = {}; + + const migrationLogs = new AggregatedLog(); + + tree.write('apps/myapp/remix.config.js', remixConfig); + tree.write('apps/myapp/package.json', `{"type": "module"}`); + + // ACT + const target = buildPostTargetTransformer(migrationLogs)( + targetConfiguration, + tree, + { projectName: 'myapp', root: 'apps/myapp' }, + inferredTargetConfiguration + ); + + // ASSERT + const configFile = tree.read('apps/myapp/remix.config.js', 'utf-8'); + expect(configFile).toMatchInlineSnapshot(` + "import { createWatchPaths } from '@nx/remix'; + import { dirname } from 'path'; + import { fileURLToPath } from 'url'; + + const __dirname = dirname(fileURLToPath(import.meta.url)); + + /** + * @type {import('@remix-run/dev').AppConfig} + */ + export default { + ignoredRouteFiles: ['**/.*'], + // appDirectory: "app", + // assetsBuildDirectory: "public/build", + // serverBuildPath: "build/index.js", + // publicPath: "/build/", + watchPaths: () => createWatchPaths(__dirname), + };" + `); + expect(target).toMatchInlineSnapshot(` + { + "configurations": { + "dev": {}, + }, + "options": {}, + } + `); + }); +}); + +const remixConfig = `import { createWatchPaths } from '@nx/remix'; +import { dirname } from 'path'; +import { fileURLToPath } from 'url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); + +/** + * @type {import('@remix-run/dev').AppConfig} + */ +export default { + ignoredRouteFiles: ['**/.*'], + // appDirectory: "app", + // assetsBuildDirectory: "public/build", + // serverBuildPath: "build/index.js", + // publicPath: "/build/", + watchPaths: () => createWatchPaths(__dirname), +};`; diff --git a/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.ts b/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.ts index 6f563a8cf16a2..e44c781f1273d 100644 --- a/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.ts +++ b/packages/remix/src/generators/convert-to-inferred/lib/build-post-target-transformer.ts @@ -1,13 +1,7 @@ import { type Tree, type TargetConfiguration } from '@nx/devkit'; import { AggregatedLog } from '@nx/devkit/src/generators/plugin-migrations/aggregate-log-util'; -import { addConfigValuesToConfigFile, getConfigFilePath } from './utils'; -import { - processTargetOutputs, - toProjectRelativePath, -} from '@nx/devkit/src/generators/plugin-migrations/plugin-migration-utils'; -import { tsquery } from '@phenomnomnominal/tsquery'; - -type RemixConfigValues = { outputPath?: string }; +import { getConfigFilePath } from './utils'; +import { processTargetOutputs } from '@nx/devkit/src/generators/plugin-migrations/plugin-migration-utils'; export function buildPostTargetTransformer(migrationLogs: AggregatedLog) { return ( @@ -17,9 +11,6 @@ export function buildPostTargetTransformer(migrationLogs: AggregatedLog) { inferredTargetConfiguration: TargetConfiguration ) => { const remixConfigPath = getConfigFilePath(tree, projectDetails.root); - const configValues: Record = { - default: {}, - }; if (target.options) { handlePropertiesFromTargetOptions( @@ -27,7 +18,6 @@ export function buildPostTargetTransformer(migrationLogs: AggregatedLog) { target.options, projectDetails.projectName, projectDetails.root, - configValues['default'], migrationLogs ); } @@ -35,19 +25,13 @@ export function buildPostTargetTransformer(migrationLogs: AggregatedLog) { if (target.configurations) { for (const configurationName in target.configurations) { const configuration = target.configurations[configurationName]; - handlePropertiesFromTargetOptions( tree, configuration, projectDetails.projectName, projectDetails.root, - configValues[configurationName], migrationLogs ); - - if (Object.keys(configuration).length === 0) { - delete target.configurations[configurationName]; - } } if (Object.keys(target.configurations).length === 0) { @@ -75,14 +59,6 @@ export function buildPostTargetTransformer(migrationLogs: AggregatedLog) { }); } - setRemixConfigToUseConfigValueForOutput( - tree, - remixConfigPath, - projectDetails.projectName, - migrationLogs - ); - addConfigValuesToConfigFile(tree, remixConfigPath, configValues); - return target; }; } @@ -92,14 +68,14 @@ function handlePropertiesFromTargetOptions( options: any, projectName: string, projectRoot: string, - configValues: RemixConfigValues, migrationLogs: AggregatedLog ) { if ('outputPath' in options) { - configValues.outputPath = toProjectRelativePath( - options.outputPath, - projectRoot - ); + migrationLogs.addLog({ + project: projectName, + executorName: '@nx/remix:build', + log: "Unable to migrate 'outputPath'. The Remix Config will contain the locations the build artifact will be output to.", + }); delete options.outputPath; } @@ -127,56 +103,9 @@ function handlePropertiesFromTargetOptions( migrationLogs.addLog({ project: projectName, executorName: '@nx/remix:build', - log: "Unable to migrate `generateLockfile` to Remix Config. Use the `@nx/dependency-checks` ESLint rule to update your project's package.json.", + log: 'Unable to migrate `generateLockfile` to Remix Config. This option is not supported.', }); delete options.generateLockfile; } } - -function setRemixConfigToUseConfigValueForOutput( - tree: Tree, - configFilePath: string, - projectName: string, - migrationLogs: AggregatedLog -) { - const configFileContents = tree.read(configFilePath, 'utf-8'); - const ast = tsquery.ast(configFileContents); - - let startPosition = 0; - let endPosition = 0; - - const OUTPUT_PATH_SELECTOR = - 'ExportAssignment ObjectLiteralExpression PropertyAssignment:has(Identifier[name=serverBuildPath]) > StringLiteral'; - const outputPathNodes = tsquery(ast, OUTPUT_PATH_SELECTOR, { - visitAllChildren: true, - }); - if (outputPathNodes.length !== 0) { - startPosition = outputPathNodes[0].getStart(); - endPosition = outputPathNodes[0].getEnd(); - } else { - const EXPORT_CONFIG_SELECTOR = 'ExportAssignment ObjectLiteralExpression'; - const configNodes = tsquery(ast, EXPORT_CONFIG_SELECTOR, { - visitAllChildren: true, - }); - if (configNodes.length === 0) { - migrationLogs.addLog({ - project: projectName, - executorName: '@nx/remix:build', - log: 'Unable to update Remix Config to set `serverBuildPath` to custom `outputPath` found in project.json. Please update this manually.', - }); - return; - } else { - startPosition = configNodes[1].getStart(); - endPosition = startPosition; - } - } - - tree.write( - configFilePath, - `${configFileContents.slice( - 0, - startPosition - )}options.outputPath${configFileContents.slice(endPosition)}` - ); -} diff --git a/packages/remix/src/generators/convert-to-inferred/lib/serve-post-target-transformer.ts b/packages/remix/src/generators/convert-to-inferred/lib/serve-post-target-transformer.ts index ff786534966d7..7594b0ca18083 100644 --- a/packages/remix/src/generators/convert-to-inferred/lib/serve-post-target-transformer.ts +++ b/packages/remix/src/generators/convert-to-inferred/lib/serve-post-target-transformer.ts @@ -28,10 +28,6 @@ export function servePostTargetTransformer(migrationLogs: AggregatedLog) { projectDetails.projectName, projectDetails.root ); - - if (Object.keys(configuration).length === 0) { - delete target.configurations[configurationName]; - } } if (Object.keys(target.configurations).length === 0) { @@ -64,14 +60,16 @@ function handlePropertiesFromTargetOptions( } if ('port' in options) { - options.env.PORT = options.port; + options.env ??= {}; + options.env.PORT = `${options.port}`; delete options.port; } for (const [prevKey, newKey] of Object.entries(REMIX_PROPERTY_MAPPINGS)) { if (prevKey in options) { - options[newKey] = options[prevKey]; + let prevValue = options[prevKey]; delete options[prevKey]; + options[newKey] = prevValue; } } } diff --git a/packages/remix/src/generators/convert-to-inferred/lib/utils.ts b/packages/remix/src/generators/convert-to-inferred/lib/utils.ts index 8e8973602a93a..3dedcf5aa8e2e 100644 --- a/packages/remix/src/generators/convert-to-inferred/lib/utils.ts +++ b/packages/remix/src/generators/convert-to-inferred/lib/utils.ts @@ -17,42 +17,3 @@ export function getConfigFilePath(tree: Tree, root: string) { joinPathFragments(root, `remix.config.mjs`), ].find((f) => tree.exists(f)); } - -export function addConfigValuesToConfigFile( - tree: Tree, - configFile: string, - configValues: Record> -) { - const IMPORT_PROPERTY_SELECTOR = 'ImportDeclaration'; - const configFileContents = tree.read(configFile, 'utf-8'); - - const ast = tsquery.ast(configFileContents); - // AST TO GET SECTION TO APPEND TO - const importNodes = tsquery(ast, IMPORT_PROPERTY_SELECTOR, { - visitAllChildren: true, - }); - let startPosition = 0; - if (importNodes.length !== 0) { - const lastImportNode = importNodes[importNodes.length - 1]; - startPosition = lastImportNode.getEnd(); - } - - const configValuesString = ` - // These options were migrated by @nx/remix:convert-to-inferred from the project.json file. - const configValues = ${JSON.stringify(configValues)}; - - // Determine the correct configValue to use based on the configuration - const nxConfiguration = process.env.NX_TASK_TARGET_CONFIGURATION ?? 'default'; - - const options = { - ...configValues.default, - ...(configValues[nxConfiguration] ?? {}) - }`; - - tree.write( - configFile, - `${configFileContents.slice(0, startPosition)} - ${configValuesString} - ${configFileContents.slice(startPosition)}` - ); -}