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

feat(Server): add /invalidate endpoint #1546

Closed
wants to merge 1 commit into from

Conversation

camertron
Copy link

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Sadly no. How should I go about testing this? Happy to work something up given some direction.

Motivation / Use-Case

We're trying to use docker-compose for local development of a giant monolithic rails app that uses webpack for several pages written with react. For performance reasons, we're mounting a volume inside the rails container using NFS, which doesn't support file system events. Consequently, making changes to our react components doesn't trigger an automatic webpack refresh. This PR is an attempt to expose and endpoint that, when hit, will trigger a recompile. The idea is to run a separate container with a regular 'ol non-NFS volume that will listen for file system events and ping the /invalidate endpoint. Actually, while I have you here, I've been trying to figure out an easy way to get the list of files webpack-dev-server is watching, but that appears to be pretty well concealed. Any tips?

Additional Info

I added this change by creating a branch from master, but, if this change is accepted, I would love it if it could be released as both a 2.x and a 3.x version bump, since we are still stuck on 2.x at the moment.

@jsf-clabot
Copy link

jsf-clabot commented Oct 25, 2018

CLA assistant check
All committers have signed the CLA.

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.

Good idea, we need add tests for this

@alexander-akait
Copy link
Member

Actually, while I have you here, I've been trying to figure out an easy way to get the list of files webpack-dev-server is watching, but that appears to be pretty well concealed.

Here you can get all assets https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L119, maybe problem can be solved using hooks #1509 (webpack-serve deprecated, but you can try use hooksm if it is solved problem we can implement their).

Anyway adding invalidate route doesn't bad idea.

@camertron
Copy link
Author

Awesome, thanks @evilebottnawi :) Happy to work on tests for this, which test files should I look at specifically?

@alexander-akait
Copy link
Member

@camertron
Copy link
Author

Great, thanks @evilebottnawi. Is there a standard expectation I can use that asserts that compilation is triggered?

@alexander-akait
Copy link
Member

@camertron Just add your route in test, it is ensure what all works as expected

@codecov
Copy link

codecov bot commented Nov 1, 2018

Codecov Report

Merging #1546 into master will increase coverage by 0.54%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1546      +/-   ##
==========================================
+ Coverage   89.19%   89.73%   +0.54%     
==========================================
  Files           9        9              
  Lines         611      614       +3     
  Branches      186      186              
==========================================
+ Hits          545      551       +6     
+ Misses         54       52       -2     
+ Partials       12       11       -1     
Impacted Files Coverage Δ
lib/Server.js 86.17% <100.00%> (+0.84%) ⬆️

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 4c52153...56792b7. Read the comment docs.

@camertron
Copy link
Author

@evilebottnawi added a test a few days ago. No rush, but any idea when this can be merged/released?

@alexander-akait
Copy link
Member

@camertron can we rebase master? Also wait review from other contributors, feel free to ping if no one will do this in the next 3 days

@camertron camertron force-pushed the invalidate_endpoint branch from 2866974 to 5d283cc Compare November 7, 2018 05:43
@camertron
Copy link
Author

@evilebottnawi done :)

@alexander-akait
Copy link
Member

@camertron hm, something wrong with travis 😕

lib/Server.js Outdated
@@ -243,6 +243,11 @@ function Server (compiler, options = {}, _log) {
res.end('</body></html>');
});

app.get('/invalidate', (req, res) => {
Copy link
Member

Choose a reason for hiding this comment

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

Do you need func params here?

Copy link
Author

Choose a reason for hiding this comment

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

I need res, but req can be removed. Done :)

@alexander-akait alexander-akait modified the milestones: 3.2.0, 3.4.0 Apr 8, 2019
@hiroppy
Copy link
Member

hiroppy commented Apr 29, 2019

@camertron Could you rebase?

@camertron camertron requested a review from hiroppy as a code owner April 30, 2019 14:31
@camertron camertron force-pushed the invalidate_endpoint branch from da14978 to 56792b7 Compare April 30, 2019 14:45
@camertron
Copy link
Author

@evilebottnawi done :)

@hiroppy hiroppy removed the request for review from michael-ciniawsky April 30, 2019 14:55
@hiroppy
Copy link
Member

hiroppy commented May 12, 2019

@camertron Could you rebase?

@camertron
Copy link
Author

@hiroppy I just tried but I no longer understand how Server.js is architected 😢

@hiroppy
Copy link
Member

hiroppy commented May 12, 2019

Currently, Routes are separated from Server.js. See https://github.com/webpack/webpack-dev-server/blob/master/lib/utils/routes.js

@alexander-akait alexander-akait modified the milestones: 3.4.0, 3.5.0 May 16, 2019
@alexander-akait alexander-akait modified the milestones: 3.5.0, 3.7.0 Jun 6, 2019
@EslamHiko EslamHiko mentioned this pull request Apr 1, 2020
6 tasks
@camertron
Copy link
Author

@evilebottnawi I got tired of the constant requests to rebase, but I'm sad this has been closed. It could have been merged a loooong time ago, back when I originally submitted it. I'm not sure why it wasn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants