Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(middleware): expose the memory filesystem (response.locals.fs) #337

Merged
merged 3 commits into from
Sep 7, 2018
Merged

feat(middleware): expose the memory filesystem (response.locals.fs) #337

merged 3 commits into from
Sep 7, 2018

Conversation

ferdinando-ferreira
Copy link
Contributor

What kind of change does this PR introduce?
feature

Did you add tests for your changes?
yes

Summary
In order for the next middleware in line to be able to use the memory FileSystem created by webpack-dev-middleware (and access the files generated in memory by the compilation process) this commit passes the context.fs to the res.locals along with webpackStats when ServerSideRender is set to true.

Does this PR introduce a breaking change?
No

Other information
This is a PR is contingent on ServerSideRender not being removed from the middleware

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Looks good can we create test for this?

@ferdinando-ferreira
Copy link
Contributor Author

If you mean a test in the npm run test suit it was created and is present in the same commit. Now if you mean a fully featured test like it was done for #336 I already did it, you can test it with

git clone --single-branch -b samples2 https://github.com/ferdinando-ferreira/webpack-dev-middleware.git
cd webpack-dev-middleware
npm install
cd samples
cd vue-ssr
npm install
npm run dev

It is the exact same vuejs SSR example.

@codecov
Copy link

codecov bot commented Aug 30, 2018

Codecov Report

Merging #337 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #337   +/-   ##
=======================================
  Coverage   96.82%   96.82%           
=======================================
  Files           7        7           
  Lines         252      252           
=======================================
  Hits          244      244           
  Misses          8        8
Impacted Files Coverage Δ
lib/middleware.js 94.33% <100%> (+0.1%) ⬆️
lib/fs.js 95.12% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 610e260...876e519. Read the comment docs.

@alexander-akait
Copy link
Member

@ferdinando-ferreira we need add test to avoid removing this in future (accidentally)

@michael-ciniawsky michael-ciniawsky changed the title Pass the memory filesystem to the next middleware along with webpackStats feat(middleware): expose the memory filesystem to other middlewares (locals.fs) Aug 30, 2018
@michael-ciniawsky
Copy link
Member

@ferdinando-ferreira Could you please add docs about this? (e.g to the README's SSR example)

@ferdinando-ferreira
Copy link
Contributor Author

ferdinando-ferreira commented Sep 4, 2018

Could you please add docs about this? (e.g to the README's SSR example)

@michael-ciniawsky: done.

@ferdinando-ferreira
Copy link
Contributor Author

@michael-ciniawsky: everything OK with the proposed changes?

@michael-ciniawsky michael-ciniawsky changed the title feat(middleware): expose the memory filesystem to other middlewares (locals.fs) feat(middleware): expose the memory filesystem to other middlewares (response.locals.fs) Sep 7, 2018
@michael-ciniawsky michael-ciniawsky merged commit f9a138e into webpack:master Sep 7, 2018
@michael-ciniawsky michael-ciniawsky changed the title feat(middleware): expose the memory filesystem to other middlewares (response.locals.fs) feat(middleware): expose the memory filesystem (response.locals.fs) Sep 7, 2018
@alexander-akait
Copy link
Member

@michael-ciniawsky looks we should do minor release

@ferdinando-ferreira ferdinando-ferreira deleted the simplified branch September 7, 2018 12:23
@michael-ciniawsky
Copy link
Member

Released in v3.3.0 🎉

@michael-ciniawsky michael-ciniawsky removed this from the 3.4.0 milestone Sep 10, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants