diff --git a/packages/rstack/src/fmt/discovery.ts b/packages/rstack/src/fmt/discovery.ts index ebfed7e2..aedb6f58 100644 --- a/packages/rstack/src/fmt/discovery.ts +++ b/packages/rstack/src/fmt/discovery.ts @@ -1,26 +1,17 @@ -import { getFileInfo, type FileInfoOptions } from 'prettier'; import { resolveFmtOptions } from './config.ts'; import { discoverFmtPaths } from './discoverPaths.ts'; import { createFmtIgnoreMatcher } from './ignore.ts'; +import { resolveFmtParser } from './parser.ts'; +import { createFmtPluginResolver, type FmtPluginResolver } from './plugins.ts'; import type { DiscoverFmtFilesOptions, FmtFileRequest, ResolvedFmtConfig } from './types.ts'; -const fileInfoOptions = { - ignorePath: [], - resolveConfig: false, - withNodeModules: true, -} satisfies FileInfoOptions; - const resolveFileRequest = async ( filePath: string, config: ResolvedFmtConfig, + resolvePlugins: FmtPluginResolver, ): Promise => { - const options = resolveFmtOptions(filePath, config); - - if (options.plugins?.length) { - throw new Error('Prettier plugins are not supported yet.'); - } - - const parser = options.parser ?? (await getFileInfo(filePath, fileInfoOptions)).inferredParser; + const options = resolvePlugins(resolveFmtOptions(filePath, config)); + const parser = await resolveFmtParser(filePath, options); if (!parser) { return; } @@ -50,8 +41,9 @@ const discoverFmtFiles = async ({ const filePaths = isFmtIgnored ? candidates.filter((filePath) => !isFmtIgnored(filePath)) : candidates; + const resolvePlugins = createFmtPluginResolver(config.rootPath); const files = await Promise.all( - filePaths.map((filePath) => resolveFileRequest(filePath, config)), + filePaths.map((filePath) => resolveFileRequest(filePath, config, resolvePlugins)), ); return files.filter((file): file is FmtFileRequest => file !== undefined); diff --git a/packages/rstack/src/fmt/format.ts b/packages/rstack/src/fmt/format.ts index dcdb44ef..ee5dc560 100644 --- a/packages/rstack/src/fmt/format.ts +++ b/packages/rstack/src/fmt/format.ts @@ -1,5 +1,7 @@ -import { format, formatWithCursor, getFileInfo } from 'prettier'; +import { format, formatWithCursor } from 'prettier'; import { resolveFmtOptions } from './config.ts'; +import { resolveFmtParser } from './parser.ts'; +import { createFmtPluginResolver } from './plugins.ts'; import type { FormatTextOptions, FormatTextResult } from './types.ts'; /** Formats source text without reading formatter config or ignore files. */ @@ -7,21 +9,8 @@ const formatText = async ( source: string, { filePath, cursorOffset, config }: FormatTextOptions, ): Promise => { - const options = resolveFmtOptions(filePath, config); - - if (options.plugins?.length) { - throw new Error('Prettier plugins are not supported yet.'); - } - - const parser = - options.parser ?? - ( - await getFileInfo(filePath, { - ignorePath: [], - resolveConfig: false, - withNodeModules: true, - }) - ).inferredParser; + const options = createFmtPluginResolver(config.rootPath)(resolveFmtOptions(filePath, config)); + const parser = await resolveFmtParser(filePath, options); if (!parser) { return { diff --git a/packages/rstack/src/fmt/parser.ts b/packages/rstack/src/fmt/parser.ts new file mode 100644 index 00000000..dd38decc --- /dev/null +++ b/packages/rstack/src/fmt/parser.ts @@ -0,0 +1,22 @@ +import { getFileInfo, type FileInfoOptions, type Options as PrettierOptions } from 'prettier'; + +const fileInfoOptions = { + ignorePath: [], + resolveConfig: false, + withNodeModules: true, +} satisfies FileInfoOptions; + +/** Uses the configured parser or infers one without loading Prettier config. */ +const resolveFmtParser = async ( + filePath: string, + options: PrettierOptions, +): Promise => + options.parser ?? + ( + await getFileInfo(filePath, { + ...fileInfoOptions, + plugins: options.plugins, + }) + ).inferredParser; + +export { resolveFmtParser }; diff --git a/packages/rstack/src/fmt/plugins.ts b/packages/rstack/src/fmt/plugins.ts index d2843ca7..466cbf15 100644 --- a/packages/rstack/src/fmt/plugins.ts +++ b/packages/rstack/src/fmt/plugins.ts @@ -2,76 +2,68 @@ import { isAbsolute, join, resolve as resolvePath } from 'node:path'; import { pathToFileURL } from 'node:url'; import { moduleResolve } from 'import-meta-resolve'; import type { Options as PrettierOptions } from 'prettier'; -import type { ResolvedFmtConfig } from './types.ts'; +import type { FmtPluginSpecifier } from './types.ts'; type FmtPlugin = NonNullable[number]; +type FmtPluginResolver = (options: PrettierOptions) => PrettierOptions; const resolveModuleUrl = (specifier: string, parentUrl: URL): string => moduleResolve(specifier, parentUrl).href; -const resolvePlugin = (plugin: FmtPlugin, rootPath: string, parentUrl: URL): FmtPlugin => { - if (plugin instanceof URL) { - return resolveModuleUrl(plugin.href, parentUrl); - } - if (typeof plugin !== 'string') { - return plugin; - } - if (isAbsolute(plugin)) { - return resolveModuleUrl(pathToFileURL(plugin).href, parentUrl); - } - if (URL.canParse(plugin)) { - return resolveModuleUrl(plugin, parentUrl); - } +const isFmtPluginSpecifier = (plugin: FmtPlugin): plugin is FmtPluginSpecifier => + typeof plugin === 'string' || plugin instanceof URL; - try { - return resolveModuleUrl(pathToFileURL(resolvePath(rootPath, plugin)).href, parentUrl); - } catch { - return resolveModuleUrl(plugin, parentUrl); - } -}; - -const resolveOptionsPlugins = ( - options: PrettierOptions, - rootPath: string, - parentUrl: URL, -): PrettierOptions => { - const { plugins } = options; - if (!plugins?.some((plugin) => typeof plugin === 'string' || plugin instanceof URL)) { - return options; - } - - const resolvedPlugins = plugins.map((plugin) => resolvePlugin(plugin, rootPath, parentUrl)); - - return resolvedPlugins.every((plugin, index) => plugin === plugins[index]) - ? options - : { ...options, plugins: resolvedPlugins }; -}; - -/** Resolves plugin specifiers from the Rstack config root. */ -const resolveFmtConfigPlugins = (config: ResolvedFmtConfig): ResolvedFmtConfig => { - const { rootPath } = config; +/** Creates a project-root resolver for plugins in final per-file options. */ +const createFmtPluginResolver = (rootPath: string): FmtPluginResolver => { const parentUrl = pathToFileURL(join(rootPath, 'index.js')); - const baseOptions = resolveOptionsPlugins(config.baseOptions, rootPath, parentUrl); - let overrides = config.overrides; + const cache = new Map(); - for (let index = 0; index < overrides.length; index++) { - const override = overrides[index]; - if (!override.options) { - continue; + const resolvePlugin = (plugin: FmtPluginSpecifier): string => { + const specifier = plugin instanceof URL ? plugin.href : plugin; + const cached = cache.get(specifier); + if (cached !== undefined) { + return cached; } - const options = resolveOptionsPlugins(override.options, rootPath, parentUrl); - if (options !== override.options) { - if (overrides === config.overrides) { - overrides = [...overrides]; + let resolved: string; + if (isAbsolute(specifier)) { + resolved = resolveModuleUrl(pathToFileURL(specifier).href, parentUrl); + } else if (URL.canParse(specifier)) { + resolved = resolveModuleUrl(specifier, parentUrl); + } else { + try { + resolved = resolveModuleUrl( + pathToFileURL(resolvePath(rootPath, specifier)).href, + parentUrl, + ); + } catch { + resolved = resolveModuleUrl(specifier, parentUrl); } - overrides[index] = { ...override, options }; } - } - return baseOptions === config.baseOptions && overrides === config.overrides - ? config - : { ...config, baseOptions, overrides }; + cache.set(specifier, resolved); + return resolved; + }; + + return (options) => { + const { plugins } = options; + if (!plugins?.length) { + return options; + } + if (!plugins.every(isFmtPluginSpecifier)) { + // Imported plugin objects are not planned for support. + throw new TypeError( + 'Prettier plugin objects are not supported. Use a package name, path, or URL instead.', + ); + } + + const resolvedPlugins = plugins.map(resolvePlugin); + + return resolvedPlugins.every((plugin, index) => plugin === plugins[index]) + ? options + : { ...options, plugins: resolvedPlugins }; + }; }; -export { resolveFmtConfigPlugins }; +export { createFmtPluginResolver }; +export type { FmtPluginResolver }; diff --git a/packages/rstack/src/fmt/runner.ts b/packages/rstack/src/fmt/runner.ts index 439be645..f1d08049 100644 --- a/packages/rstack/src/fmt/runner.ts +++ b/packages/rstack/src/fmt/runner.ts @@ -66,7 +66,7 @@ const runFmtFilesParallel = async ( } }; -/** Checks every worker payload before any formatting can begin. */ +/** Checks whether every request can be cloned for a worker. */ const canRunFmtFilesParallel = (files: FmtFileRequest[]): boolean => { try { structuredClone(files); diff --git a/packages/rstack/src/fmt/types.ts b/packages/rstack/src/fmt/types.ts index 8254ce00..770f1625 100644 --- a/packages/rstack/src/fmt/types.ts +++ b/packages/rstack/src/fmt/types.ts @@ -1,6 +1,21 @@ import type { Config as PrettierConfig, Options as PrettierOptions } from 'prettier'; -interface FmtConfig extends PrettierConfig { +/** Plugin objects cannot cross worker boundaries and are not planned for support. */ +type FmtPluginSpecifier = string | URL; + +type FmtOptions = Omit & { + plugins?: FmtPluginSpecifier[]; +}; + +type PrettierOverride = NonNullable[number]; + +type FmtOverride = Omit & { + options?: FmtOptions; +}; + +interface FmtConfig extends Omit { + plugins?: FmtPluginSpecifier[]; + overrides?: FmtOverride[]; /** Gitignore-compatible patterns relative to the Rstack config root. */ ignorePatterns?: string[]; } @@ -103,6 +118,7 @@ export type { FmtFileResult, FmtFileRequest, FmtMode, + FmtPluginSpecifier, FmtRunResult, FormatTextOptions, FormatTextResult, diff --git a/packages/rstack/tests/cli/fmt/index.test.ts b/packages/rstack/tests/cli/fmt/index.test.ts index 038b7ba1..0cd7ea97 100644 --- a/packages/rstack/tests/cli/fmt/index.test.ts +++ b/packages/rstack/tests/cli/fmt/index.test.ts @@ -15,6 +15,20 @@ const writeProjectFile = (filePath: string, content: string): void => { const readProjectFile = (filePath: string): string => readFileSync(path.join(projectPath, filePath), 'utf8'); +const writeFixturePlugin = (): void => { + writeProjectFile( + 'node_modules/prettier-plugin-fixture/package.json', + JSON.stringify({ name: 'prettier-plugin-fixture', exports: './index.mjs' }), + ); + writeProjectFile( + 'node_modules/prettier-plugin-fixture/index.mjs', + `export default { + languages: [{ name: 'Fixture JSON', parsers: ['json'], extensions: ['.fixture'] }], +}; +`, + ); +}; + const runCLI = (args: string[]) => { const env: NodeJS.ProcessEnv = { ...process.env, NO_COLOR: '1' }; delete env.FORCE_COLOR; @@ -216,23 +230,79 @@ test('returns exit code 2 for config errors', () => { expect(result.stderr).toContain('invalid fmt config'); }); -test('returns exit code 2 for unsupported plugins', () => { +test.each([ + ['parallel execution', []], + ['serial execution', ['--no-parallel']], +] as const)('formats with a project-local plugin using %s', (_, options) => { writeProjectFile( 'rstack.config.ts', `import { define } from 'rstack'; define.fmt({ - plugins: ['prettier-plugin-example'], + plugins: ['prettier-plugin-fixture'], }); `, ); - writeProjectFile('index.ts', 'const message="hello"'); + writeFixturePlugin(); + writeProjectFile('first.fixture', '{"first":true}'); + writeProjectFile('second.fixture', '{"second":true}'); + + const result = runFmt([...options, '*.fixture']); + + expect(result.status).toBe(0); + expect(result.stdout).toBe('first.fixture\nsecond.fixture\n'); + expect(result.stderr).toBe(''); + expect(readProjectFile('first.fixture')).toBe('{ "first": true }\n'); + expect(readProjectFile('second.fixture')).toBe('{ "second": true }\n'); +}); + +test('formats mixed plugin overrides in parallel', () => { + writeProjectFile( + 'rstack.config.ts', + `import { define } from 'rstack'; + +define.fmt({ + overrides: [ + { + files: '*.fixture', + options: { plugins: ['prettier-plugin-fixture'] }, + }, + ], +}); +`, + ); + writeFixturePlugin(); + writeProjectFile('data.fixture', '{"value":true}'); + writeProjectFile('index.ts', 'const value=true'); + + const result = runFmt(['data.fixture', 'index.ts']); + + expect(result.status).toBe(0); + expect(result.stdout).toBe('data.fixture\nindex.ts\n'); + expect(result.stderr).toBe(''); + expect(readProjectFile('data.fixture')).toBe('{ "value": true }\n'); + expect(readProjectFile('index.ts')).toBe('const value = true;\n'); +}); + +test('returns exit code 2 for imported plugin objects', () => { + writeProjectFile( + 'rstack.config.ts', + `import { define } from 'rstack'; + +define.fmt({ + plugins: [{ languages: [] }], +}); +`, + ); + writeProjectFile('index.ts', 'const value=true'); const result = runFmt(['index.ts']); expect(result.status).toBe(2); expect(result.stdout).toBe(''); - expect(result.stderr).toContain('Prettier plugins are not supported yet.'); + expect(result.stderr).toContain( + 'Prettier plugin objects are not supported. Use a package name, path, or URL instead.', + ); }); test('returns exit code 2 for formatting errors', () => { diff --git a/packages/rstack/tests/fmt/discovery.test.ts b/packages/rstack/tests/fmt/discovery.test.ts index e90fb400..4e9b6aeb 100644 --- a/packages/rstack/tests/fmt/discovery.test.ts +++ b/packages/rstack/tests/fmt/discovery.test.ts @@ -1,5 +1,6 @@ import { mkdirSync } from 'node:fs'; import path from 'node:path'; +import { pathToFileURL } from 'node:url'; import { expect, test } from 'rstack/test'; import { normalizeFmtConfig } from '../../src/fmt/config.ts'; import { discoverFmtFiles } from '../../src/fmt/discovery.ts'; @@ -63,3 +64,45 @@ test('resolves parsers and accepts unknown extensions with an explicit parser', }); }); }); + +test('resolves plugins after applying matching overrides', async () => { + await withTempProject(async (rootPath) => { + const pluginEntry = writeProjectFile( + rootPath, + 'node_modules/prettier-plugin-fixture/index.mjs', + `export default { + languages: [{ name: 'Fixture JSON', parsers: ['json'], extensions: ['.fixture'] }], +}; +`, + ); + writeProjectFile( + rootPath, + 'node_modules/prettier-plugin-fixture/package.json', + JSON.stringify({ name: 'prettier-plugin-fixture', exports: './index.mjs' }), + ); + writeProjectFile(rootPath, 'example.fixture'); + writeProjectFile(rootPath, 'example.ts'); + const config = { + overrides: [ + { + files: '*.fixture', + options: { plugins: ['prettier-plugin-fixture'] }, + }, + { + files: '*.md', + options: { plugins: ['missing-plugin'] }, + }, + ], + }; + + const files = await discover(rootPath, ['example.fixture', 'example.ts'], config); + + expect(files).toHaveLength(2); + expect(files[0]).toMatchObject({ + options: { + parser: 'json', + plugins: [pathToFileURL(pluginEntry).href], + }, + }); + }); +}); diff --git a/packages/rstack/tests/fmt/format.test.ts b/packages/rstack/tests/fmt/format.test.ts index 1f923252..7be3703c 100644 --- a/packages/rstack/tests/fmt/format.test.ts +++ b/packages/rstack/tests/fmt/format.test.ts @@ -2,6 +2,7 @@ import path from 'node:path'; import { expect, test } from 'rstack/test'; import { normalizeFmtConfig } from '../../src/fmt/config.ts'; import { formatText } from '../../src/fmt/format.ts'; +import { withTempProject, writeProjectFile } from './helpers.ts'; const rootPath = import.meta.dirname; @@ -62,6 +63,40 @@ test('uses an explicit parser for unknown file extensions', async () => { }); }); +test('supports a plugin path from matching overrides', async () => { + await withTempProject(async (projectPath) => { + writeProjectFile( + projectPath, + 'plugins/fixture.mjs', + `export default { + languages: [{ name: 'Fixture JSON', parsers: ['json'], extensions: ['.fixture'] }], +}; +`, + ); + const config = normalizeFmtConfig( + { + overrides: [ + { + files: '*.fixture', + options: { plugins: ['./plugins/fixture.mjs'] }, + }, + ], + }, + projectPath, + ); + + const result = await formatText('{"value":true}', { + config, + filePath: path.join(projectPath, 'example.fixture'), + }); + + expect(result).toEqual({ + status: 'formatted', + formatted: '{ "value": true }\n', + }); + }); +}); + test('formats an explicitly provided node_modules file', async () => { const config = normalizeFmtConfig(undefined, rootPath); diff --git a/packages/rstack/tests/fmt/plugins.test.ts b/packages/rstack/tests/fmt/plugins.test.ts index ec81c133..cac6da5b 100644 --- a/packages/rstack/tests/fmt/plugins.test.ts +++ b/packages/rstack/tests/fmt/plugins.test.ts @@ -1,7 +1,6 @@ import { pathToFileURL } from 'node:url'; import { expect, test } from 'rstack/test'; -import { normalizeFmtConfig } from '../../src/fmt/config.ts'; -import { resolveFmtConfigPlugins } from '../../src/fmt/plugins.ts'; +import { createFmtPluginResolver } from '../../src/fmt/plugins.ts'; import { withTempProject, writeProjectFile } from './helpers.ts'; test('resolves plugin specifiers from the config root', async () => { @@ -31,56 +30,33 @@ test('resolves plugin specifiers from the config root', async () => { const relativePlugin = writeProjectFile(rootPath, 'plugins/relative.mjs'); const absolutePlugin = writeProjectFile(rootPath, 'plugins/absolute.mjs'); const urlPlugin = writeProjectFile(rootPath, 'plugins/url.mjs'); - const overridePlugin = writeProjectFile(rootPath, 'plugins/override.mjs'); - const pluginObject = { languages: [] }; - const config = normalizeFmtConfig( - { - plugins: [ - 'prettier-plugin-packagejson', - './plugins/relative.mjs', - absolutePlugin, - pathToFileURL(urlPlugin), - 'data:text/javascript,export default {}', - pluginObject, - ], - overrides: [ - { - files: '*.json', - options: { - plugins: ['plugins/override.mjs'], - }, - }, - ], - }, - rootPath, - ); + const options = { + plugins: [ + 'prettier-plugin-packagejson', + './plugins/relative.mjs', + absolutePlugin, + pathToFileURL(urlPlugin), + 'data:text/javascript,export default {}', + ], + }; - const resolved = resolveFmtConfigPlugins(config); + const resolved = createFmtPluginResolver(rootPath)(options); - expect(resolved.baseOptions.plugins).toEqual([ + expect(resolved.plugins).toEqual([ pathToFileURL(packageEntry).href, pathToFileURL(relativePlugin).href, pathToFileURL(absolutePlugin).href, pathToFileURL(urlPlugin).href, 'data:text/javascript,export default {}', - pluginObject, ]); - expect(resolved.baseOptions.plugins?.at(-1)).toBe(pluginObject); - expect(resolved.overrides[0].options?.plugins).toEqual([pathToFileURL(overridePlugin).href]); - expect(config.baseOptions.plugins?.[0]).toBe('prettier-plugin-packagejson'); - expect(config.overrides[0].options?.plugins?.[0]).toBe('plugins/override.mjs'); + expect(options.plugins[0]).toBe('prettier-plugin-packagejson'); }); }); -test('does not copy config containing only plugin objects', () => { - const pluginObject = { languages: [] }; - const config = normalizeFmtConfig( - { - plugins: [pluginObject], - overrides: [{ files: '*.json', options: { plugins: [pluginObject] } }], - }, - import.meta.dirname, - ); +test('rejects imported plugin objects', () => { + const options = { plugins: [{ languages: [] }] }; - expect(resolveFmtConfigPlugins(config)).toBe(config); + expect(() => createFmtPluginResolver(import.meta.dirname)(options)).toThrow( + 'Prettier plugin objects are not supported. Use a package name, path, or URL instead.', + ); }); diff --git a/packages/rstack/tests/fmt/runnerParallelPreflight.test.ts b/packages/rstack/tests/fmt/runnerParallelPreflight.test.ts index 415c3363..afcf28ce 100644 --- a/packages/rstack/tests/fmt/runnerParallelPreflight.test.ts +++ b/packages/rstack/tests/fmt/runnerParallelPreflight.test.ts @@ -6,6 +6,7 @@ import { withTempProject, writeProjectFile } from './helpers.ts'; const mocks = rs.hoisted(() => ({ createFmtWorkerCalls: [] as [number, number | undefined][], + formatFileSerialCalls: [] as FmtFileRequest[], })); rs.mock('../../src/fmt/parallel.ts', () => ({ @@ -15,8 +16,16 @@ rs.mock('../../src/fmt/parallel.ts', () => ({ }, })); +rs.mock('../../src/fmt/serial.ts', () => ({ + formatFileSerial: (file: FmtFileRequest) => { + mocks.formatFileSerialCalls.push(file); + return Promise.resolve(true); + }, +})); + beforeEach(() => { mocks.createFmtWorkerCalls.length = 0; + mocks.formatFileSerialCalls.length = 0; }); const createRequest = ( @@ -31,7 +40,7 @@ const createRequest = ( }, }); -test('does not write files when worker startup fails', async () => { +test('sends plugin URL requests to workers before writing', async () => { await withTempProject(async (rootPath) => { const filePaths = ['first.ts', 'second.ts'].map((name) => writeProjectFile(rootPath, name, 'const value=1'), @@ -39,7 +48,9 @@ test('does not write files when worker startup fails', async () => { await expect( runFmtFiles({ - files: filePaths.map((filePath) => createRequest(filePath)), + files: filePaths.map((filePath) => + createRequest(filePath, ['file:///prettier-plugin-fixture.mjs']), + ), mode: 'write', cache: false, parallel: true, @@ -48,6 +59,7 @@ test('does not write files when worker startup fails', async () => { ).rejects.toThrow('worker startup failed'); expect(mocks.createFmtWorkerCalls).toEqual([[2, 3]]); + expect(mocks.formatFileSerialCalls).toEqual([]); for (const filePath of filePaths) { expect(readFileSync(filePath, 'utf8')).toBe('const value=1'); @@ -57,24 +69,26 @@ test('does not write files when worker startup fails', async () => { test('uses serial execution when options cannot be cloned', async () => { await withTempProject(async (rootPath) => { - const pluginWithFunction = { - languages: [], - run() {}, - }; const filePaths = ['first.ts', 'second.ts'].map((name) => writeProjectFile(rootPath, name, 'const value=1'), ); + const files = filePaths.map((filePath) => createRequest(filePath)); + for (const file of files) { + Object.assign(file.options, { customOption() {} }); + } const result = await runFmtFiles({ - files: filePaths.map((filePath) => createRequest(filePath, [pluginWithFunction])), + files, mode: 'write', cache: false, parallel: true, }); expect(result.files.map((file) => file.status)).toEqual(['written', 'written']); + expect(mocks.createFmtWorkerCalls).toEqual([]); + expect(mocks.formatFileSerialCalls).toEqual(files); for (const filePath of filePaths) { - expect(readFileSync(filePath, 'utf8')).toBe('const value = 1;\n'); + expect(readFileSync(filePath, 'utf8')).toBe('const value=1'); } }); });