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(jest): Include RWJS_DEBUG_ENV in jest web env #9065

Merged
merged 4 commits into from
Sep 2, 2023

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented Aug 25, 2023

Fixes the issue described here: https://community.redwoodjs.com/t/redwood-v6-0-0-upgrade-guide/5044/70

Note that I'm unable to reproduce the actual failure, but it looks like a legitimate error to me (and I understand why).

@dac09 dac09 added the release:fix This PR is a fix label Aug 25, 2023
@dac09 dac09 added this to the next-release-patch milestone Aug 25, 2023
@dac09 dac09 requested a review from jtoar August 25, 2023 08:32
@thedavidprice
Copy link
Contributor

@Josh-Walker-GM any chance you could review this one and help get it in for patch?

@Josh-Walker-GM
Copy link
Collaborator

I can't reproduce the issue and so I don't yet understand what effect the changes here would have. The change to the jest preset looks good but I don't understand the change to the DevFatalErrorPage. Is that just to clean up invalid usage of globalThis and the jest change is the primary change here that fixes things?

This isn't my area of strength so I would be more than happy to defer to @dac09 if you think this is the fix.

@dac09
Copy link
Contributor Author

dac09 commented Aug 31, 2023

I can't reproduce the issue and so I don't yet understand what effect the changes here would have.

Same for me, but I think the issue would happen if you made your Layout or Page error out, so that it bubbles up to the nearest error boundary (which in the User's case was the FatalErrorBoundary).

the jest change is the primary change here that fixes things?

Yes, the jest change should fix the error. In v6, we actually shipped a slightly different version of the code in DevFatalErrorPage.

The globalThis change is because although technically globalThis is valid, Webpack wouldn't replace the var if you used globalThis.SOME_VARIABLE_DEFINED_IN_WEBPACK - just being extra careful.

@jtoar jtoar modified the milestones: next-release-patch, v6.1.1 Sep 2, 2023
@jtoar
Copy link
Contributor

jtoar commented Sep 2, 2023

deferring to you both on this and merging

@jtoar jtoar merged commit e90ae0c into redwoodjs:main Sep 2, 2023
jtoar pushed a commit that referenced this pull request Sep 2, 2023
Fixes the issue described here:
https://community.redwoodjs.com/t/redwood-v6-0-0-upgrade-guide/5044/70

Note that I'm unable to reproduce the actual failure, but it looks like
a legitimate error to me (and I understand why).
jtoar pushed a commit that referenced this pull request Sep 2, 2023
Fixes the issue described here:
https://community.redwoodjs.com/t/redwood-v6-0-0-upgrade-guide/5044/70

Note that I'm unable to reproduce the actual failure, but it looks like
a legitimate error to me (and I understand why).
@dac09 dac09 deleted the fix/webside-test-failures branch September 2, 2023 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants