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

Defer throwing asset errors until after dependencies are handled. #2475

Merged
44 changes: 44 additions & 0 deletions packages/core/integration-tests/test/hmr.js
Expand Up @@ -529,4 +529,48 @@ describe('hmr', function() {

await buildEnd;
});

it('should watch new dependencies that cause errors', async function() {
await ncp(
path.join(__dirname, '/integration/elm'),
path.join(__dirname, '/input')
);

b = bundler(path.join(__dirname, '/input/index.js'), {
watch: true,
hmr: true
});
await b.bundle();

ws = new WebSocket('ws://localhost:' + b.options.hmrPort);

const buildEnd = nextEvent(b, 'buildEnd');

await sleep(100);
ncp(
path.join(__dirname, '/input/src/MainWithBrokenDep.elm'),
path.join(__dirname, '/input/src/Main.elm')
);

let msg = JSON.parse(await nextEvent(ws, 'message'));
assert.equal(msg.type, 'error');

await sleep(100);
fs.writeFile(
path.join(__dirname, '/input/src/BrokenDep.elm'),
`
module BrokenDep exposing (anError)


anError : String
anError =
"2"
`
);

msg = JSON.parse(await nextEvent(ws, 'message'));
assert.equal(msg.type, 'error-resolved');

await buildEnd;
});
});
@@ -0,0 +1,8 @@
module BrokenDep exposing (anError)

{- This module causes a compiler error -}


anError : String
anError =
2
@@ -0,0 +1,48 @@
module Main exposing (main)

import BrokenDep
import Browser
import Html exposing (Html, button, div, text)
import Html.Events exposing (onClick)


type alias Model =
{ count : Int }


initialModel : Model
initialModel =
{ count = 0 }


type Msg
= Increment
| Decrement


update : Msg -> Model -> Model
update msg model =
case msg of
Increment ->
{ model | count = model.count + 1 }

Decrement ->
{ model | count = model.count - 1 }


view : Model -> Html Msg
view model =
div []
[ button [ onClick Increment ] [ text "+1" ]
, div [] [ text <| String.fromInt model.count ]
, button [ onClick Decrement ] [ text "-1" ]
]


main : Program () Model Msg
main =
Browser.sandbox
{ init = initialModel
, view = view
, update = update
}
7 changes: 7 additions & 0 deletions packages/core/parcel-bundler/src/Bundler.js
Expand Up @@ -593,6 +593,13 @@ class Bundler extends EventEmitter {
})
);

// If there was a processing error, re-throw now that we've set up
// depdenency watchers. This keeps reloading working if there is an
// error in a dependency not directly handled by Parcel.
if (processed.error !== null) {
throw processed.error;
}

// Store resolved assets in their original order
dependencies.forEach((dep, i) => {
asset.dependencies.set(dep.name, dep);
Expand Down
13 changes: 10 additions & 3 deletions packages/core/parcel-bundler/src/Pipeline.js
Expand Up @@ -17,16 +17,23 @@ class Pipeline {
}

let asset = this.parser.getAsset(path, options);
let generated = await this.processAsset(asset);
let error = null;
let generatedMap = {};
for (let rendition of generated) {
generatedMap[rendition.type] = rendition.value;
try {
let generated = await this.processAsset(asset);
for (let rendition of generated) {
generatedMap[rendition.type] = rendition.value;
}
} catch (err) {
error = err;
error.fileName = path;
}

return {
id: asset.id,
dependencies: Array.from(asset.dependencies.values()),
generated: generatedMap,
error: error,
hash: asset.hash,
cacheData: asset.cacheData
};
Expand Down
27 changes: 21 additions & 6 deletions packages/core/parcel-bundler/src/assets/ElmAsset.js
Expand Up @@ -44,12 +44,7 @@ class ElmAsset extends Asset {
options.optimize = true;
}

let compiled = await this.elm.compileToString(this.name, options);
this.contents = compiled.toString();
if (this.options.hmr) {
let {inject} = await localRequire('elm-hot', this.name);
this.contents = inject(this.contents);
}
this.elmOpts = options;
}

async collectDependencies() {
Expand All @@ -76,6 +71,13 @@ class ElmAsset extends Asset {
}

async generate() {
let compiled = await this.elm.compileToString(this.name, this.elmOpts);
this.contents = compiled.toString();
if (this.options.hmr) {
let {inject} = await localRequire('elm-hot', this.name);
this.contents = inject(this.contents);
}

let output = this.contents;

if (this.options.minify) {
Expand Down Expand Up @@ -129,6 +131,19 @@ class ElmAsset extends Asset {
return result.code;
}
}

generateErrorMessage(err) {
// For some reason, if not converted to a plain object,
// the error message is lost somewhere between Pipeline.js
// and Bundler.js.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes an issue for Elm, but I probably also need to do something like this in Pipeline.js for other asset types that throw Errors. I'm not certain why this is necessary, but I suspect that there's an object copy somewhere in WorkerFarm that causes data to be lost if Pipeline.process returns an obejct containing an Error

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixed.


// The stack is not particularly useful, but other code may
// expect it and try to print it.
return {
message: err.message,
stack: ''
};
}
}

module.exports = ElmAsset;