Skip to content
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

Exclude contextual module references from cleaning dll task #38733

Merged
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/dev/build/lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ export function assertAbsolute(path) {
}
}

export function isFileAccessible(path) {
assertAbsolute(path);

try {
fs.accessSync(path);
return true;
} catch (e) {
return false;
}
}

function longInspect(value) {
return inspect(value, {
maxArrayLength: Infinity
Expand Down
1 change: 1 addition & 0 deletions src/dev/build/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export {
deleteAll,
deleteEmptyFolders,
compress,
isFileAccessible,
} from './fs';
export { scanDelete } from './scan_delete';
export { scanCopy } from './scan_copy';
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,9 @@ export const CleanClientModulesOnDLLTask = {

// Get dll entries filtering out the ones
// from any whitelisted module
const dllEntries = await getDllEntries(dllManifestPath, whiteListedModules);
const dllEntries = await getDllEntries(dllManifestPath, whiteListedModules, baseDir);

for (const relativeEntryPath of dllEntries) {
if (relativeEntryPath.includes('@elastic/eui')) {
continue;
}

const entryPath = `${baseDir}/${relativeEntryPath}`;

// Clean a module included into the dll
Expand Down
23 changes: 19 additions & 4 deletions src/dev/build/tasks/nodejs_modules/webpack_dll.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@
* under the License.
*/

import { deleteAll, read, write } from '../../lib';
import { dirname, sep, relative } from 'path';
import { deleteAll, isFileAccessible, read, write } from '../../lib';
import { dirname, sep, relative, resolve } from 'path';
import pkgUp from 'pkg-up';
import globby from 'globby';

export async function getDllEntries(manifestPath, whiteListedModules) {
function checkDllEntryAccess(entry, baseDir = '') {
const resolvedPath = baseDir ? resolve(baseDir, entry) : entry;
return isFileAccessible(resolvedPath);
}

export async function getDllEntries(manifestPath, whiteListedModules, baseDir = '') {
const manifest = JSON.parse(await read(manifestPath));

if (!manifest || !manifest.content) {
Expand All @@ -46,7 +51,17 @@ export async function getDllEntries(manifestPath, whiteListedModules) {
const isWhiteListed = whiteListedModules.some(nonEntry => entry.includes(`node_modules${sep}${nonEntry}${sep}`));
const isNodeModule = entry.includes('node_modules');

return !isWhiteListed && isNodeModule;
// NOTE: when using dynamic imports on webpack the entry paths could be created
// with special context module (ex: lazy recursive) values over directories that are not real files
// and only exists in runtime, so we need to check if the entry is a real file.
// We found that problem through the issue https://github.com/elastic/kibana/issues/38481
//
// More info:
// https://github.com/webpack/webpack/blob/master/examples/code-splitting-harmony/README.md
// https://webpack.js.org/guides/dependency-management/#require-with-expression
const isAccessible = checkDllEntryAccess(entry, baseDir);

return !isWhiteListed && isNodeModule && isAccessible;
});
}

Expand Down
21 changes: 19 additions & 2 deletions src/dev/build/tasks/nodejs_modules/webpack_dll.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
* under the License.
*/

import { read } from '../../lib';
import { isFileAccessible, read } from '../../lib';
import { getDllEntries } from './webpack_dll';

jest.mock('../../lib', () => ({
read: jest.fn()
read: jest.fn(),
isFileAccessible: jest.fn(),
}));

const manifestContentMock = JSON.stringify({
Expand Down Expand Up @@ -49,13 +50,29 @@ describe('Webpack DLL Build Tasks Utils', () => {
it('should get dll entries correctly', async () => {
read.mockImplementationOnce(async () => manifestContentMock);

isFileAccessible.mockImplementation(() => true);

const mockManifestPath = '/mock/mock_dll_manifest.json';
const mockModulesWhitelist = [ 'dep1' ];
const dllEntries = await getDllEntries(mockManifestPath, mockModulesWhitelist);

expect(dllEntries).toEqual(expect.arrayContaining(['/mock/node_modules/dep2', '/mock/node_modules/dep3']));
});

it('should only include accessible files', async () => {
read.mockImplementationOnce(async () => manifestContentMock);

isFileAccessible.mockImplementation(() => false);

const mockManifestPath = '/mock/mock_dll_manifest.json';
const mockModulesWhitelist = [ 'dep1' ];
const dllEntries = await getDllEntries(mockManifestPath, mockModulesWhitelist);

isFileAccessible.mockRestore();

expect(dllEntries.length).toEqual(0);
});

it('should throw an error for no manifest file', async () => {
read.mockImplementationOnce(async () => noManifestMock);

Expand Down