Skip to content

make sure wrapper is found before accessing the classList#225

Merged
shellscape merged 1 commit intoshellscape:masterfrom
serkanyersen:patch-2
Feb 1, 2022
Merged

make sure wrapper is found before accessing the classList#225
shellscape merged 1 commit intoshellscape:masterfrom
serkanyersen:patch-2

Conversation

@serkanyersen
Copy link
Contributor

When used with https://github.com/pmmmwh/react-refresh-webpack-plugin this error shows up during incremental builds.
Cannot read property 'classList' of null

Screen Shot 2021-05-20 at 3 31 01 PM

This PR contains:

  • bugfix
  • feature
  • refactor
  • tests
  • documentation
  • metadata

Breaking Changes?

  • yes
  • no

If yes, please describe the breakage.

Please Describe Your Changes

I've added a check for wrapper and if it's null, it won't try to access classList

When used with https://github.com/pmmmwh/react-refresh-webpack-plugin this error shows up during incremental builds.
```Cannot read property 'classList' of null```
@shellscape
Copy link
Owner

Incremental builds wasn't something we had in mind when the plugin was initially developed. What effect does wrapper not being yet available have on the developer experience?

@serkanyersen
Copy link
Contributor Author

Sorry this was a little rushed, I'll try to come up with a reliable repro and some more explanation tomorrow. 👍

@serkanyersen
Copy link
Contributor Author

we have actually disabled this feature because it wasn't really supposed to work together with react-refresh-webpack-plugin I think.

If you don't want this change you can close this PR 👍

@shellscape
Copy link
Owner

I'm happy to accommodate needs here. When you say disabled, do you mean you've commented it out or added an option to disable it?

@shellscape shellscape merged commit a110e81 into shellscape:master Feb 1, 2022
@shellscape
Copy link
Owner

thanks!

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.

2 participants