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

Don't discard invalidations when transformer throws an error #7547

Merged
merged 4 commits into from Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 14 additions & 4 deletions packages/core/core/src/Transformation.js
Expand Up @@ -26,9 +26,11 @@ import {Readable} from 'stream';
import nullthrows from 'nullthrows';
import logger, {PluginLogger} from '@parcel/logger';
import ThrowableDiagnostic, {
anyToDiagnostic,
errorToDiagnostic,
escapeMarkdown,
md,
type Diagnostic,
} from '@parcel/diagnostic';
import {SOURCEMAP_EXTENSIONS} from '@parcel/utils';
import {hashString} from '@parcel/hash';
Expand Down Expand Up @@ -83,7 +85,8 @@ export type TransformationOpts = {|
|};

export type TransformationResult = {|
assets: Array<AssetValue>,
assets?: Array<AssetValue>,
error?: Array<Diagnostic>,
configRequests: Array<ConfigRequest>,
invalidations: Array<RequestInvalidation>,
invalidateOnFileCreate: Array<InternalFileCreateInvalidation>,
Expand Down Expand Up @@ -178,19 +181,26 @@ export default class Transformation {
asset.value.isSource,
asset.value.pipeline,
);
let results = await this.runPipelines(pipeline, asset);
let assets = results.map(a => a.value);
let assets, error;
try {
let results = await this.runPipelines(pipeline, asset);
assets = results.map(a => a.value);
} catch (e) {
error = e;
}

let configRequests = getConfigRequests([...this.configs.values()]);
let devDepRequests = getWorkerDevDepRequests([
...this.devDepRequests.values(),
]);

// $FlowFixMe
// $FlowFixMe because of $$raw
return {
$$raw: true,
assets,
configRequests,
// When throwing an error, this (de)serialization is done automatically by the WorkerFarm
error: error ? anyToDiagnostic(error) : undefined,
invalidateOnFileCreate: this.invalidateOnFileCreate,
invalidations: [...this.invalidations.values()],
devDepRequests,
Expand Down
14 changes: 11 additions & 3 deletions packages/core/core/src/requests/AssetRequest.js
Expand Up @@ -13,6 +13,7 @@ import type {ConfigAndCachePath} from './ParcelConfigRequest';
import type {TransformationResult} from '../Transformation';

import nullthrows from 'nullthrows';
import ThrowableDiagnostic from '@parcel/diagnostic';
import {hashString} from '@parcel/hash';
import createParcelConfigRequest from './ParcelConfigRequest';
import {runDevDepRequest} from './DevDepRequest';
Expand Down Expand Up @@ -128,6 +129,7 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) {
let {
assets,
configRequests,
error,
invalidations,
invalidateOnFileCreate,
devDepRequests,
Expand All @@ -138,8 +140,10 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) {
}): TransformationResult);

let time = Date.now() - start;
for (let asset of assets) {
asset.stats.time = time;
if (assets) {
for (let asset of assets) {
asset.stats.time = time;
}
}

for (let invalidation of invalidateOnFileCreate) {
Expand Down Expand Up @@ -171,5 +175,9 @@ async function run({input, api, farm, invalidateReason, options}: RunInput) {
await runConfigRequest(api, configRequest);
}

return assets;
if (error != null) {
throw new ThrowableDiagnostic({diagnostic: error});
} else {
return nullthrows(assets);
}
}
43 changes: 43 additions & 0 deletions packages/core/integration-tests/test/cache.js
Expand Up @@ -5836,6 +5836,49 @@ describe('cache', function () {
}
});

it('properly watches included files after a transformer error', async function () {
this.timeout(15000);
let subscription;
let fixture = path.join(__dirname, '/integration/included-file');
try {
let b = bundler(path.join(fixture, 'index.txt'), {
inputFS: overlayFS,
shouldDisableCache: false,
});
await overlayFS.mkdirp(fixture);
await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'a');
subscription = await b.watch();
let event = await getNextBuild(b);
invariant(event.type === 'buildSuccess');
let output1 = await overlayFS.readFile(
event.bundleGraph.getBundles()[0].filePath,
'utf8',
);
assert.strictEqual(output1, 'a');

// Change included file
await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'ERROR');
event = await getNextBuild(b);
invariant(event.type === 'buildFailure');
assert.strictEqual(event.diagnostics[0].message, 'Custom error');

// Clear transformer error
await overlayFS.writeFile(path.join(fixture, 'included.txt'), 'b');
event = await getNextBuild(b);
invariant(event.type === 'buildSuccess');
let output3 = await overlayFS.readFile(
event.bundleGraph.getBundles()[0].filePath,
'utf8',
);
assert.strictEqual(output3, 'b');
} finally {
if (subscription) {
await subscription.unsubscribe();
subscription = null;
}
}
});

it('should support moving the project root', async function () {
// This test relies on the real filesystem because the memory fs doesn't support renames.
// But renameSync is broken on windows in CI with EPERM errors. Just skip this test for now.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.