-
Notifications
You must be signed in to change notification settings - Fork 12
[fix] Vite outDir for sourcemaps upload (take 2) #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
421684b
d1d4698
44bea9a
7e37ce5
a22a62b
7bd30be
e7835a3
c6465c9
28a9539
b297c24
c8a6c27
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
| "@dd/core": "workspace:*" | ||
| }, | ||
| "devDependencies": { | ||
| "rollup": "4.24.2", | ||
| "typescript": "5.4.3" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,237 @@ | ||||||
| // Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. | ||||||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||||||
| // Copyright 2019-Present Datadog, Inc. | ||||||
|
|
||||||
| import { addFixtureFiles } from '@dd/tests/_jest/helpers/mocks'; | ||||||
| import type { InputOptions } from 'rollup'; | ||||||
|
|
||||||
| import { computeCwd, computeOutDir, getAbsoluteOutDir, getOutDirFromOutputs } from './rollup'; | ||||||
|
|
||||||
| jest.mock('@dd/core/helpers/fs', () => { | ||||||
| const original = jest.requireActual('@dd/core/helpers/fs'); | ||||||
| return { | ||||||
| ...original, | ||||||
| existsSync: jest.fn(), | ||||||
| }; | ||||||
| }); | ||||||
|
|
||||||
| describe('Rollup Helpers', () => { | ||||||
| describe('getAbsoluteOutDir', () => { | ||||||
| const cases = [ | ||||||
| { | ||||||
| description: 'return empty string when outDir is empty', | ||||||
| cwd: '/project', | ||||||
| outDir: '', | ||||||
| expected: '', | ||||||
| }, | ||||||
| { | ||||||
| description: 'return absolute path when outDir is already absolute', | ||||||
| cwd: '/project', | ||||||
| outDir: '/absolute/path/dist', | ||||||
| expected: '/absolute/path/dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'resolve relative path against cwd', | ||||||
| cwd: '/project', | ||||||
| outDir: 'dist', | ||||||
| expected: '/project/dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'resolve relative path with parent directory', | ||||||
| cwd: '/project/src', | ||||||
| outDir: '../dist', | ||||||
| expected: '/project/dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'resolve relative path with current directory', | ||||||
| cwd: '/project', | ||||||
| outDir: './dist', | ||||||
| expected: '/project/dist', | ||||||
| }, | ||||||
| ]; | ||||||
|
|
||||||
| test.each(cases)('Should $description', ({ cwd, outDir, expected }) => { | ||||||
| expect(getAbsoluteOutDir(cwd, outDir)).toBe(expected); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('getOutDirFromOutputs', () => { | ||||||
| const cases = [ | ||||||
| { | ||||||
| description: 'return empty string when outputOptions is undefined', | ||||||
| outputOptions: undefined, | ||||||
| expected: '', | ||||||
| }, | ||||||
| { | ||||||
| description: 'extract dir from single output object with dir', | ||||||
| outputOptions: { dir: 'dist' }, | ||||||
| expected: 'dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'extract dir from single output object with file', | ||||||
| outputOptions: { file: 'dist/bundle.js' }, | ||||||
| expected: 'dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'extract dir from array of outputs with dir', | ||||||
| outputOptions: [{ dir: 'dist' }, { dir: 'dist2' }], | ||||||
| expected: 'dist2', | ||||||
| }, | ||||||
| { | ||||||
| description: 'extract dir from array of outputs with file', | ||||||
| outputOptions: [{ file: 'dist/bundle.js' }, { file: 'dist2/bundle.js' }], | ||||||
| expected: 'dist2', | ||||||
| }, | ||||||
| { | ||||||
| description: 'prefer dir over file in same output', | ||||||
| outputOptions: { dir: 'dist', file: 'other/bundle.js' }, | ||||||
| expected: 'dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'handle nested file paths', | ||||||
| outputOptions: { file: 'dist/assets/js/bundle.js' }, | ||||||
| expected: 'dist/assets/js', | ||||||
| }, | ||||||
| { | ||||||
| description: 'return empty string for empty array', | ||||||
| outputOptions: [], | ||||||
| expected: '', | ||||||
| }, | ||||||
| { | ||||||
| description: 'return empty string when no dir or file specified', | ||||||
| outputOptions: [{ format: 'esm' }, { format: 'cjs' }] as any, | ||||||
| expected: '', | ||||||
| }, | ||||||
| ]; | ||||||
|
|
||||||
| test.each(cases)('Should $description', ({ outputOptions, expected }) => { | ||||||
| expect(getOutDirFromOutputs(outputOptions)).toBe(expected); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('computeOutDir', () => { | ||||||
| beforeAll(() => { | ||||||
| jest.spyOn(process, 'cwd').mockReturnValue('/base/cwd'); | ||||||
| }); | ||||||
|
|
||||||
| const cases = [ | ||||||
| { | ||||||
| description: 'handle relative output', | ||||||
| options: { | ||||||
| output: { dir: 'custom-dist/assets' }, | ||||||
| }, | ||||||
| expected: '/base/cwd/custom-dist/assets', | ||||||
| }, | ||||||
| { | ||||||
| description: 'handle absolute output', | ||||||
| options: { | ||||||
| output: { dir: '/absolute/dist' }, | ||||||
| }, | ||||||
| expected: '/absolute/dist', | ||||||
| }, | ||||||
| { | ||||||
| description: 'handle no output', | ||||||
| options: {}, | ||||||
| expected: '/base/cwd/dist', | ||||||
| }, | ||||||
| ]; | ||||||
|
|
||||||
| test.each(cases)('Should $description', ({ options, expected }) => { | ||||||
| expect(computeOutDir(options as InputOptions)).toBe(expected); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe('computeCwd', () => { | ||||||
| beforeAll(() => { | ||||||
| jest.spyOn(process, 'cwd').mockReturnValue('/base/cwd'); | ||||||
| }); | ||||||
|
|
||||||
| beforeEach(() => { | ||||||
| // Set up virtual file system for package.json files | ||||||
| addFixtureFiles({ | ||||||
| '/project/package.json': '', | ||||||
| '/project/src/package.json': '', | ||||||
| '/project/lib/package.json': '', | ||||||
| '/base/cwd/package.json': '', | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| const cases = [ | ||||||
| { | ||||||
| description: 'handle string input', | ||||||
| options: { input: '/project/src/index.js' }, | ||||||
| expected: '/project', | ||||||
| }, | ||||||
| { | ||||||
| description: 'handle array input', | ||||||
| options: { | ||||||
| input: ['/project/src/index.js', '/project/lib/util.js'], | ||||||
| }, | ||||||
| expected: '/project', | ||||||
| }, | ||||||
| { | ||||||
| description: 'handle object input', | ||||||
| options: { | ||||||
| input: { | ||||||
| main: '/project/src/index.js', | ||||||
| util: '/project/lib/util.js', | ||||||
| }, | ||||||
| }, | ||||||
| expected: '/project', | ||||||
| }, | ||||||
| { | ||||||
| description: 'throw error for invalid input type in object', | ||||||
| options: { input: { main: 123 } }, | ||||||
| shouldThrow: 'Invalid input type', | ||||||
| }, | ||||||
| { | ||||||
| description: 'throw error for invalid input type in array', | ||||||
| options: { input: [123] }, | ||||||
| shouldThrow: 'Invalid input type', | ||||||
| }, | ||||||
| { | ||||||
| description: 'include absolute output directory in cwd computation', | ||||||
| options: { | ||||||
| input: '/project/src/index.js', | ||||||
| output: { dir: '/project/dist' }, | ||||||
| }, | ||||||
| expected: '/project', | ||||||
| }, | ||||||
| { | ||||||
| description: 'ignore relative output directory', | ||||||
| options: { | ||||||
| input: '/project/src/index.js', | ||||||
| output: { dir: 'dist' }, | ||||||
| }, | ||||||
| expected: '/project', | ||||||
| }, | ||||||
| { | ||||||
| description: 'fallback to process.cwd when no input', | ||||||
| options: {}, | ||||||
| expected: '/base/cwd', | ||||||
| }, | ||||||
| { | ||||||
| description: 'fallback to process.cwd with relative input', | ||||||
| options: { input: 'index.js' }, | ||||||
| expected: '/base/cwd', | ||||||
| }, | ||||||
| ]; | ||||||
|
|
||||||
| test.each(cases)('Should $description', ({ options, expected, shouldThrow }) => { | ||||||
| const errors = []; | ||||||
| const results = []; | ||||||
| const expectedResults = expected ? [expected] : []; | ||||||
| const expectedErrors = shouldThrow ? [shouldThrow] : []; | ||||||
|
|
||||||
| try { | ||||||
| const result = computeCwd(options as InputOptions); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly here const cases = [
// ...
] satisfies Array<{ description: string; expected: string; options: InputOptions }>;
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still need it because Rollup doesn't type |
||||||
| results.push(result); | ||||||
| } catch (error: any) { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TypeScript won't let me.
|
||||||
| errors.push(error.message); | ||||||
| } | ||||||
|
|
||||||
| expect(errors).toEqual(expectedErrors); | ||||||
| expect(results).toEqual(expectedResults); | ||||||
| }); | ||||||
| }); | ||||||
| }); | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| import { getHighestPackageJsonDir, getNearestCommonDirectory } from '@dd/core/helpers/paths'; | ||
| import path from 'path'; | ||
| import type { InputOptions, OutputOptions, RollupOptions } from 'rollup'; | ||
|
|
||
| // Compute the CWD based on a list of directories. | ||
| const getCwd = (dirs: Set<string>) => { | ||
|
yoannmoinet marked this conversation as resolved.
|
||
| for (const dir of dirs) { | ||
| // eslint-disable-next-line no-undef | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
| const highestPackage = getHighestPackageJsonDir(dir); | ||
| if (highestPackage && !dirs.has(highestPackage)) { | ||
| dirs.add(highestPackage); | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
|
||
| // Fall back to the nearest common directory. | ||
| const nearestDir = getNearestCommonDirectory(Array.from(dirs)); | ||
| if (nearestDir !== path.sep) { | ||
| return nearestDir; | ||
| } | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
| }; | ||
|
|
||
| export const getAbsoluteOutDir = (cwd: string, outDir: string) => { | ||
| if (!outDir) { | ||
| return ''; | ||
| } | ||
|
|
||
| return path.isAbsolute(outDir) ? outDir : path.resolve(cwd, outDir); | ||
| }; | ||
|
|
||
| export const getOutDirFromOutputs = (outputOptions: RollupOptions['output']) => { | ||
| if (!outputOptions) { | ||
| return ''; | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| const normalizedOutputOptions = Array.isArray(outputOptions) ? outputOptions : [outputOptions]; | ||
| let outDir: string = ''; | ||
| // FIXME: This is an oversimplification, we should handle builds with multiple outputs. | ||
| // Ideally, `outDir` should only be computed for the build-report. | ||
| // And build-report should also handle multiple outputs. | ||
| for (const output of normalizedOutputOptions) { | ||
| if (output.dir) { | ||
| outDir = output.dir; | ||
| } else if (output.file) { | ||
| outDir = path.dirname(output.file); | ||
| } | ||
| } | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
|
|
||
| return outDir; | ||
| }; | ||
|
|
||
| export const computeOutDir = (options: InputOptions) => { | ||
| if ('output' in options) { | ||
| return getAbsoluteOutDir( | ||
| process.cwd(), | ||
| getOutDirFromOutputs(options.output as OutputOptions), | ||
| ); | ||
| } else { | ||
| // Fallback to process.cwd()/dist as it is rollup's default. | ||
| return path.resolve(process.cwd(), 'dist'); | ||
| } | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
| }; | ||
|
|
||
| export const computeCwd = (options: InputOptions) => { | ||
| const directoriesForCwd: Set<string> = new Set(); | ||
|
|
||
| if (options.input) { | ||
| const normalizedInput = Array.isArray(options.input) | ||
| ? options.input | ||
| : typeof options.input === 'object' | ||
| ? Object.values(options.input) | ||
| : [options.input]; | ||
|
|
||
| for (const input of normalizedInput) { | ||
| if (typeof input === 'string') { | ||
| directoriesForCwd.add(path.dirname(input)); | ||
| } else { | ||
| throw new Error('Invalid input type'); | ||
| } | ||
|
yoannmoinet marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
yoannmoinet marked this conversation as resolved.
|
||
|
|
||
| // In case an absolute path has been provided in the output options, | ||
| // we include it in the directories list for CWD computation. | ||
| if ( | ||
| 'output' in options && | ||
| path.isAbsolute(getOutDirFromOutputs(options.output as OutputOptions)) | ||
| ) { | ||
| directoriesForCwd.add(computeOutDir(options)); | ||
| } | ||
|
|
||
| const cwd = getCwd(directoriesForCwd); | ||
|
|
||
| if (cwd) { | ||
| return cwd; | ||
| } | ||
|
|
||
| // Fallbacks | ||
| return process.cwd(); | ||
|
yoannmoinet marked this conversation as resolved.
|
||
| }; | ||
Uh oh!
There was an error while loading. Please reload this page.