fix(engine-core): change console.errors to console.warns(#3692)

[yashpatel1998]

Details

change console.errors to console.warns
*fix: display warning instead of errors if the rendering continues despite the absence of lwc:dom="manual" directive

Does this pull request introduce a breaking change?

• ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

• ✅ No, it does not introduce an observable change.

[salesforce-cla]

Thanks for the contribution! Before we can merge this, we need @yashpatel1998 to sign the Salesforce Inc. Contributor License Agreement.

[abdulsattar]

@yashpatel1998 what's the motivation for this? console.error doesn't block rendering and will continue. I found the relevant issue.

Thanks for this PR! You need to accept the CLA before we merge, though.

[yashpatel1998]

@abdulsattar I have already accepted the CLA, but seems it is not reflected here

[jmsjtu]

@yashpatel1998 could you confirm that the email used to sign your commits is the same as the one listed on your GitHub account (yashpatel.dev@gmail.com)?

If they are different, it could cause issues with the CLA bot.

[yashpatel1998]

@jmsjtu I confirm it is the same email address. Will re-running the bot pipeline fix the issue by any chance?

[nolanlawson]

@yashpatel1998 You could try pushing a new commit to this branch?

[nolanlawson]

BTW this will almost certainly have some broken tests once the CI runs... if you do cd packages/@lwc/integration-karma && yarn test, you will see the test failures.

[yashpatel1998]

@nolanlawson Pushed the integration-karma test changes. Could you check if the changes look good now?
A side note, the tests were failing for me in development mode with below error but succeeded for production. Not sure if this is intended.

[nolanlawson]

@yashpatel1998 If the tests are failing in development mode, it looks like it's because there is a console.warn/error that is not being accounted for. The test are designed such that, if you forget to add the correct expect().toLog*Dev wrapper, then it will fail the tests overall.

Thank you again for the PR!

[yashpatel1998]

@nolanlawson Got you. Let me look into it and push the fix. Thank you for the help ! :)

[yashpatel1998]

@nolanlawson Lastest commit should fix the failing tests. Let me know if there are changes to be done.

[nolanlawson]

LGTM. +1 when the CI tests are passing.

Thank you @yashpatel1998 !! 🎉

[nolanlawson]

This doesn't fully resolve #3692 but it does fix probably the biggest source of spurious console errors.

[yashpatel1998]

@nolanlawson Do you want me to replace other console.errors as part of the same issue later?

Excited for my first successful contribution to the framework! 😁

[nolanlawson]

@yashpatel1998 Sure, those can be done in a separate PR. :) Thanks again for the contribution!

[nolanlawson]

Integration tests are failing but they look unrelated to this PR. Might need to check if something changed in Chrome or something.

[yashpatel1998]

Yeah, seems like.
Sure. I will take a look at the new PR,

[nolanlawson]

Thanks very much!