Skip to content

fix: add writeToDisk support for clean: true#1472

Closed
Mortaro wants to merge 1 commit intowebpack:mainfrom
Mortaro:fix-write-to-disk-cleanup
Closed

fix: add writeToDisk support for clean: true#1472
Mortaro wants to merge 1 commit intowebpack:mainfrom
Mortaro:fix-write-to-disk-cleanup

Conversation

@Mortaro
Copy link
Copy Markdown

@Mortaro Mortaro commented Mar 6, 2023

This PR contains a:

  • [x ] bugfix
  • new feature
  • code refactor
  • [x ] test update
  • documentation update
  • typo fix
  • metadata update

Motivation / Use-Case

I'm the author of Nullstack, one of its build modes targets chrome extensions.
I use the writeToDisk option in development mode to preview the extensions with HMR, but the middleware was never cleaning up the real FS when MemFS unlinked files, causing the output folder to become bloated over time.

Breaking Changes

There are no breaking changes.

Additional Info

The solution I added might be a bit hacky, but it does solve this issue that has been open for a minute: #861

I have temporarily moved the unstable branch of Nullstack to use my fork, but would love to have this fix merged so i can rollback to the official package 💜

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 6, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Mortaro / name: Christian Mortaro (ff90a3a)

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 6, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.39%. Comparing base (6f7b8da) to head (ff90a3a).
⚠️ Report is 616 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1472      +/-   ##
==========================================
+ Coverage   97.34%   97.39%   +0.04%     
==========================================
  Files           9        9              
  Lines         377      384       +7     
  Branches      112      114       +2     
==========================================
+ Hits          367      374       +7     
  Misses          9        9              
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

/** @type {String} */ originalPath,
/** @type {(arg0?: null | NodeJS.ErrnoException) => void} */ originalCallback
) => {
fs.unlink(originalPath, () => {});
Copy link
Copy Markdown
Member

@alexander-akait alexander-akait Mar 13, 2023

Choose a reason for hiding this comment

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

Hello, there is racing you should wait the first operation finished, also using fs.unlink is not safe, because some developer use custom outputFileSystem to avoid file removing, but we remove them and break application, we nee to think deeply how we can handle such case. In theiry we can apply clean plugin before watch and it will with original fs

@Mapuppy09
Copy link
Copy Markdown

Add write disk support for clean true

@alexander-akait
Copy link
Copy Markdown
Member

Closing due to inactivity.

@Mapuppy09
Copy link
Copy Markdown

Add clean disk

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.

3 participants