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. #2468

Conversation

domenkozar
Copy link

@domenkozar domenkozar commented Dec 26, 2018

Testing #2344 with following changes:

  • removed whitespace
  • rebased on top of master

(lost original authorship of the commit, hopefully we can fix this before merging)

This allows compiled langauges like Elm that handle their own dependency
compilation to set up watchers for dependencies so that hot-reload
continues to work due to errors in new depdenencies. Fixes parcel-bundler#2147.
Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

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

Tests are failing in CI. could you try to resolve this?

@MattCheely
Copy link

FWIW, I made some changes to my original branch that got my local test runs in a consistent state with master (not everything succeeds, but the same things fail), but It looks like the addition of elm-hot has mooted the need for this fix for Elm assets, although it could still be useful for other languages. It does mean my primary test app is no longer useful for testing though.

@domenkozar
Copy link
Author

@MattCheely I still get the same bug, why do you think elm-hot changed anything?

@MattCheely
Copy link

MattCheely commented Dec 27, 2018

@domenkozar 🤦‍♂️ Looks like you are right and it is still an issue. I was working on this at a meetup last night and during testing with a local up-to-date parcel install, it seemed like things were working, but reviewing things now, I must have made a mistake somewhere in that verification.

I can look at this some more tonight - it seems the latest changes in my branch are also no longer fixing the issue, but until I look closer, I'm not sure whether I introduced the problem when fixing test failures or it's related to the elm-hot changes. Feel free to take a look in the meantime if you have the time and inclination.

@MattCheely
Copy link

@domenkozar I think I have resolved all of my previous issues in #2475. I think we can close this PR, if that's OK with you.

@domenkozar domenkozar closed this Dec 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants