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

Fix(transformer/elm): handle elm error properly without errors property #8319

Conversation

Shinyaigeek
Copy link
Contributor

↪️ Pull Request

Fixes: #8263

Hi team! long time to see.

I investigated #8263 issue, and I noticed this bug is caused because error object thrown by compileToString module does not necessarily have .errors property. I make a fix to handle such a case.

💻 Examples

use Debug.log and build it with production mode.

🚨 Test instructions

make a unit test to check such a case.

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@Shinyaigeek Shinyaigeek force-pushed the fix/handle-elm-error--without-errors-property-properly branch from 7ae283a to 7f918ba Compare July 18, 2022 15:47
Copy link
Member

@mischnic mischnic left a comment

Choose a reason for hiding this comment

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

Thanks!

@devongovett
Copy link
Member

Is this still needed now that #8076 is merged? I tried the test and it seemed to pass against v2.

@Shinyaigeek
Copy link
Contributor Author

Oh, sure. As @devongovett said, this change is not needed because #8076 merged. The same as this change is done in https://github.com/parcel-bundler/parcel/pull/8076/files#diff-087bd115f321d7345bbe411da8a4734cf827c1af5af408b74e206f9cd485d305R73-R76.

@Shinyaigeek
Copy link
Contributor Author

Shinyaigeek commented Aug 1, 2022

so I closed this PR. anyway, thank you for review and help!

@Shinyaigeek Shinyaigeek closed this Aug 1, 2022
@devongovett
Copy link
Member

No worries! Thanks for the PR, and sorry for the duplication.

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.

Elm code with Debug.log fails with Javascript error
3 participants