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.

Sign up for GitHub

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

Jump to bottom

check this.compiler.outputFileSystem.constructor is not undefined #447

Merged
merged 3 commits into from
May 17, 2021
Merged

check this.compiler.outputFileSystem.constructor is not undefined #447

merged 3 commits into from
May 17, 2021

Conversation

kedarv
Copy link
Contributor

@kedarv kedarv commented May 13, 2021

via #384 (comment)

In some cases, this.compiler.outputFileSystem.constructor can be undefined, causing access to properties on this to fail. This PR adds a check to prevent this.

@kedarv kedarv mentioned this pull request May 13, 2021
Closed
@alexander-akait
Copy link
Member

/cc @valscion Can we add tests late? I can send tests late, right now I am busy with webpack-dev-server

@valscion
Copy link
Member

Yeah we can add tests later to get this bug fix out.

I'll copy over what I wrote in #384 (comment) to here:

as long as there are no tests, it's possible that we'll break the fix even in a patch release as there is no way we can make sure that this fix doesn't suddenly stop working unless we have test coverage for that.

@alexander-akait
Copy link
Member

@valscion It looks like I'll have to waste time on a pretty obvious bug 😕 😞

@valscion
Copy link
Member

I'll get this bug fix out, no worries about that. I'm merely noting the effect a missing test coverage has for this kind of bug fix.

It's not possible for us to keep this bug fix working for sure if we do changes to the code later. Reproducing the bug takes effort we're not willing to do every time we change the code.

@valscion
Copy link
Member

I do appreciate @alexander-akait you reproducing the original issue in #384! It was very helpful ☺️ it makes it possible for us to ship this fix even without test coverage, and I do appreciate that 💞

@valscion valscion merged commit 2bcf474 into webpack-contrib:master May 17, 2021
@alexander-akait
Copy link
Member

alexander-akait commented May 17, 2021 •
edited
Loading

Can you try using this #384 (comment)? Do you see the error? Just want check it out too

@valscion
Copy link
Member

Yes I see the error with the reproduction

vesa_Vesas-MBP____code_muut_webpack-bundle-analyzer_issue-384-repro

@valscion
Copy link
Member

This fix has been released in v4.4.2 🎉

jkzing pushed a commit to jkzing/umi-webpack-bundle-analyzer that referenced this pull request Jun 24, 2021
@kedarv @valscion @jkzing
check this.compiler.outputFileSystem.constructor is not undefined (we…
835181a 
…bpack-contrib#447)

* check this.compiler.outputFileSystem.constructor is not undefined

* Fix trailing spaces lint

* Update changelog with bug fix

Co-authored-by: Vesa Laakso <[email protected]>
@jkzing jkzing mentioned this pull request Jun 24, 2021
Merged
RaoHai added a commit to umijs/umi-webpack-bundle-analyzer that referenced this pull request Jun 24, 2021
@RaoHai
Merge pull request #14 from jkzing/fix/constructor-name
1f22409 
fix: check this.compiler.outputFileSystem.constructor is not undefined (webpack-contrib#447)
deanshub added a commit to wix-incubator/webpack-bundle-analyzer that referenced this pull request Aug 19, 2021
@shlior7 @deanshub
@valscion @dependabot @kedarv @th0r
npm publish and upstream fetch (#2)
d531e2a 
* Fix tests timeout (webpack-contrib#435)

* Fix tests timeout

* wip

* increasing timeout

* Update test/plugin.js

Co-authored-by: Vesa Laakso <[email protected]>

* increasing timeout

Co-authored-by: Vesa Laakso <[email protected]>

* Fixes webpack-contrib#417 missing module chunks (webpack-contrib#433)

* Fixes webpack-contrib#417 missing module chunks

* triger build

* test fix

* test fix

* Fix changelog username link texts

* v4.4.1

* Bump url-parse from 1.4.7 to 1.5.1 (webpack-contrib#443)

Bumps [url-parse](https://github.com/unshiftio/url-parse) from 1.4.7 to 1.5.1.
- [Release notes](https://github.com/unshiftio/url-parse/releases)
- [Commits](unshiftio/url-parse@1.4.7...1.5.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump postcss from 8.1.6 to 8.2.10 (webpack-contrib#446)

Bumps [postcss](https://github.com/postcss/postcss) from 8.1.6 to 8.2.10.
- [Release notes](https://github.com/postcss/postcss/releases)
- [Changelog](https://github.com/postcss/postcss/blob/main/CHANGELOG.md)
- [Commits](postcss/postcss@8.1.6...8.2.10)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* check this.compiler.outputFileSystem.constructor is not undefined (webpack-contrib#447)

* check this.compiler.outputFileSystem.constructor is not undefined

* Fix trailing spaces lint

* Update changelog with bug fix

Co-authored-by: Vesa Laakso <[email protected]>

* v4.4.2

* Update deps

* Remove unused `exports-loader`

* Bump hosted-git-info from 2.8.8 to 2.8.9 (webpack-contrib#445)

Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.8 to 2.8.9.
- [Release notes](https://github.com/npm/hosted-git-info/releases)
- [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md)
- [Commits](npm/hosted-git-info@v2.8.8...v2.8.9)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump browserslist from 4.14.6 to 4.16.6 in /test/webpack-versions/5.4.0 (webpack-contrib#450)

Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.14.6 to 4.16.6.
- [Release notes](https://github.com/browserslist/browserslist/releases)
- [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md)
- [Commits](browserslist/browserslist@4.14.6...4.16.6)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump dns-packet from 1.3.1 to 1.3.4 (webpack-contrib#451)

Bumps [dns-packet](https://github.com/mafintosh/dns-packet) from 1.3.1 to 1.3.4.
- [Release notes](https://github.com/mafintosh/dns-packet/releases)
- [Changelog](https://github.com/mafintosh/dns-packet/blob/master/CHANGELOG.md)
- [Commits](mafintosh/dns-packet@v1.3.1...v1.3.4)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump ws from 7.4.5 to 7.4.6 (webpack-contrib#452)

Bumps [ws](https://github.com/websockets/ws) from 7.4.5 to 7.4.6.
- [Release notes](https://github.com/websockets/ws/releases)
- [Commits](websockets/ws@7.4.5...7.4.6)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump normalize-url from 4.5.0 to 4.5.1 (webpack-contrib#455)

Bumps [normalize-url](https://github.com/sindresorhus/normalize-url) from 4.5.0 to 4.5.1.
- [Release notes](https://github.com/sindresorhus/normalize-url/releases)
- [Commits](https://github.com/sindresorhus/normalize-url/commits)

---
updated-dependencies:
- dependency-name: normalize-url
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump ssri from 6.0.1 to 6.0.2 in /test/webpack-versions/4.44.2 (webpack-contrib#441)

Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2.
- [Release notes](https://github.com/npm/ssri/releases)
- [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md)
- [Commits](npm/ssri@v6.0.1...v6.0.2)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump elliptic from 6.5.3 to 6.5.4 in /test/webpack-versions/4.44.2 (webpack-contrib#421)

Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.3 to 6.5.4.
- [Release notes](https://github.com/indutny/elliptic/releases)
- [Commits](indutny/elliptic@v6.5.3...v6.5.4)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* npm publish

* npm publish

* lint fix

* npmpublic

* name-change

* test title change

* yarnrc

* fixed yarnrc

Co-authored-by: Dean Shub <[email protected]>
Co-authored-by: Vesa Laakso <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kedar <[email protected]>
Co-authored-by: th0r <[email protected]>
@ludofischer ludofischer mentioned this pull request Sep 21, 2021
Merged
@moul-bot moul-bot mentioned this pull request Oct 2, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
flag: Needs Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kedarv @alexander-akait @valscion