Added webpack dynamic import routes to the router. #742
Conversation
|
|
||
| this.router.get('/_webpack/:number', async (req, res, params) => { | ||
| if (isNaN(params.number)) throw new Error('Webpack dynamic imports should be numbered') | ||
| const p = join(this.dir, `.next/${params.number}`) |
There was a problem hiding this comment.
This should be join(this.dir, '.next', params.number) ?
There was a problem hiding this comment.
Sure, I'll change it
| @@ -80,6 +80,13 @@ export default class Server { | |||
| const p = join(__dirname, '..', 'client', ...(params.path || [])) | |||
| await serveStatic(req, res, p) | |||
| }) | |||
|
|
|||
| this.router.get('/_webpack/:number', async (req, res, params) => { | |||
There was a problem hiding this comment.
This path seems to conflict with https://github.com/zeit/next.js/blob/master/server/build/webpack.js#L179
There was a problem hiding this comment.
In what case?
There was a problem hiding this comment.
I also am not sure exactly when this path is used, but some files, like patch files of HMR, are served from it.
There was a problem hiding this comment.
As far as I read #728, is this intentional ? Anyway it would be better to use a same logic to serve these files on dev and production IMO.
There was a problem hiding this comment.
I think here _webpack is the path we exposed as publicPath in the config.
I think in the dev mode, this is implemented by the webpack dev server itself. We only need to do this on production.
There was a problem hiding this comment.
So, shall we add this route only in the production mode? So we are sure, we don't interfere with the webpack dev server.
There was a problem hiding this comment.
So with an if ( dev ) this should be ok @nkzawa ?
There was a problem hiding this comment.
Any thoughts?
| @@ -80,6 +80,13 @@ export default class Server { | |||
| const p = join(__dirname, '..', 'client', ...(params.path || [])) | |||
| await serveStatic(req, res, p) | |||
| }) | |||
|
|
|||
| this.router.get('/_webpack/:number', async (req, res, params) => { | |||
| if (isNaN(params.number)) throw new Error('Webpack dynamic imports should be numbered') | |||
There was a problem hiding this comment.
We should respond with appropriate status code (404) ?
|
We are working on something similar on the |
|
@arunoda should we close this then? |
|
YEAH! |
Fixes #728
(and it's pretty cool)