-
-
Notifications
You must be signed in to change notification settings - Fork 396
feat: add experimental autoIncludeExternalSources option for monorepo support #1826
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
501f32a
8a069a5
d47859f
3db87e4
c256d95
d632e5c
0895d28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. These tests don't actually test the new behavior. Please actually include a external file in project, then extract and make sure the are included. Here's an example of how to unzip and check for file existence: https://github.com/johnnyfekete/wxt/blob/d632e5c3cdd96421662b791c6d6008b32dde666e/packages/wxt/e2e/tests/zip.test.ts#L103-L123 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. You're right, I changed them. It was a lot more complicated than I thought, so maybe you can think of a nicer way to test files that are outside of the project folder, but I'm using the zip test functionalities that you recommended. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,262 @@ | ||
import { describe, it, expect, vi, beforeEach } from 'vitest'; | ||
import { gatherExternalFiles } from '../external-files'; | ||
import { BuildOutput, OutputChunk } from '../../../types'; | ||
import fs from 'fs-extra'; | ||
import path from 'node:path'; | ||
import { setFakeWxt } from '../testing/fake-objects'; | ||
|
||
// Mock fs-extra | ||
vi.mock('fs-extra'); | ||
const mockFs = vi.mocked(fs); | ||
|
||
describe('gatherExternalFiles', () => { | ||
beforeEach(() => { | ||
vi.clearAllMocks(); | ||
|
||
// Setup fake wxt instance with default config | ||
setFakeWxt({ | ||
config: { | ||
zip: { | ||
sourcesRoot: '/project/src', | ||
}, | ||
logger: { | ||
info: vi.fn(), | ||
debug: vi.fn(), | ||
}, | ||
}, | ||
}); | ||
}); | ||
|
||
it('should return empty array when no external files are found', async () => { | ||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: [ | ||
'/project/src/background.ts', | ||
'/project/src/utils.ts', | ||
], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([]); | ||
}); | ||
|
||
it('should include external files that exist outside the project directory', async () => { | ||
const externalFile = '/parent/shared/utils.ts'; | ||
|
||
// Mock fs.access to succeed for external file | ||
mockFs.access.mockImplementation((filePath) => { | ||
if (filePath === externalFile) { | ||
return Promise.resolve(); | ||
} | ||
return Promise.reject(new Error('File not found')); | ||
}); | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: ['/project/src/background.ts', externalFile], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([externalFile]); | ||
expect(mockFs.access).toHaveBeenCalledWith(externalFile); | ||
}); | ||
|
||
it('should exclude files in node_modules', async () => { | ||
const nodeModuleFile = '/project/node_modules/some-package/index.js'; | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: ['/project/src/background.ts', nodeModuleFile], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([]); | ||
expect(mockFs.access).not.toHaveBeenCalledWith(nodeModuleFile); | ||
}); | ||
|
||
it('should exclude virtual modules', async () => { | ||
const virtualModule = 'virtual:wxt-background'; | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: ['/project/src/background.ts', virtualModule], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([]); | ||
expect(mockFs.access).not.toHaveBeenCalledWith(virtualModule); | ||
}); | ||
|
||
it('should exclude HTTP URLs', async () => { | ||
const httpUrl = 'http://example.com/script.js'; | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: ['/project/src/background.ts', httpUrl], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([]); | ||
expect(mockFs.access).not.toHaveBeenCalledWith(httpUrl); | ||
}); | ||
|
||
it('should skip non-existent external files', async () => { | ||
const nonExistentFile = '/parent/missing/file.ts'; | ||
|
||
// Mock fs.access to reject for non-existent file | ||
mockFs.access.mockRejectedValue(new Error('File not found')); | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: ['/project/src/background.ts', nonExistentFile], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([]); | ||
expect(mockFs.access).toHaveBeenCalledWith(nonExistentFile); | ||
}); | ||
|
||
it('should handle multiple external files and deduplicate them', async () => { | ||
const externalFile1 = '/parent/shared/utils.ts'; | ||
const externalFile2 = '/parent/shared/types.ts'; | ||
|
||
// Mock fs.access to succeed for both external files | ||
mockFs.access.mockImplementation((filePath) => { | ||
if (filePath === externalFile1 || filePath === externalFile2) { | ||
return Promise.resolve(); | ||
} | ||
return Promise.reject(new Error('File not found')); | ||
}); | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: [ | ||
'/project/src/background.ts', | ||
externalFile1, | ||
externalFile2, | ||
externalFile1, // Duplicate should be ignored | ||
], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toHaveLength(2); | ||
expect(result).toContain(externalFile1); | ||
expect(result).toContain(externalFile2); | ||
}); | ||
|
||
it('should only process chunk-type outputs', async () => { | ||
const externalFile = '/parent/shared/utils.ts'; | ||
|
||
const buildOutput: BuildOutput = { | ||
manifest: { manifest_version: 3, name: 'test', version: '1.0.0' }, | ||
publicAssets: [], | ||
steps: [ | ||
{ | ||
chunks: [ | ||
{ | ||
type: 'asset', | ||
fileName: 'icon.png', | ||
}, | ||
{ | ||
type: 'chunk', | ||
fileName: 'background.js', | ||
moduleIds: [externalFile], | ||
} as OutputChunk, | ||
], | ||
entrypoints: [], | ||
}, | ||
], | ||
}; | ||
|
||
// Mock fs.access to succeed | ||
mockFs.access.mockResolvedValue(undefined); | ||
|
||
const result = await gatherExternalFiles(buildOutput); | ||
expect(result).toEqual([externalFile]); | ||
expect(mockFs.access).toHaveBeenCalledOnce(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, if this is the right place to add the documentation 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, probably fine.