-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Sourcemap and Breakpoint Fixes #3121
Conversation
35aaa21
to
d2817ef
Compare
server/render.js
Outdated
const devBuildId = Date.now() | ||
// Hard coding build id to development to allow for breakpoints to be set across | ||
// page reloads. | ||
const devBuildId = 'development' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this bring the mentioned issue #2014 back?
The disable cache feature in dev tools is on by default. Also cache
management in dev mode is something devs should be aware of. It was one of
the first things I learned back in the “old days” :)
…On Tue, Oct 17, 2017 at 1:43 PM Vlad Frolov ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In server/render.js
<#3121 (comment)>:
> @@ -93,11 +93,9 @@ async function doRender (req, res, pathname, query, {
}
const docProps = await loadGetInitialProps(Document, { ...ctx, renderPage })
- // While developing, we should not cache any assets.
- // So, we use a different buildId for each page load.
- // With that we can ensure, we have unique URL for assets per every page load.
- // So, it'll prevent issues like this: https://git.io/vHLtb
- const devBuildId = Date.now()
+ // Hard coding build id to development to allow for breakpoints to be set across
+ // page reloads.
+ const devBuildId = 'development'
Won't this bring the mentioned issue #2014
<#2014> back?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3121 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAL_Jpju2Pt__PkVrF1FRwTADOk6VLYLks5stPUSgaJpZM4P8Z1r>
.
|
But headers also provide better fixes that allow both to live together.
…On Tue, Oct 17, 2017 at 2:36 PM Kevin Decker ***@***.***> wrote:
The disable cache feature in dev tools is on by default. Also cache
management in dev mode is something devs should be aware of. It was one of
the first things I learned back in the “old days” :)
On Tue, Oct 17, 2017 at 1:43 PM Vlad Frolov ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In server/render.js
> <#3121 (comment)>:
>
> > @@ -93,11 +93,9 @@ async function doRender (req, res, pathname, query, {
> }
>
> const docProps = await loadGetInitialProps(Document, { ...ctx, renderPage })
> - // While developing, we should not cache any assets.
> - // So, we use a different buildId for each page load.
> - // With that we can ensure, we have unique URL for assets per every page load.
> - // So, it'll prevent issues like this: https://git.io/vHLtb
> - const devBuildId = Date.now()
> + // Hard coding build id to development to allow for breakpoints to be set across
> + // page reloads.
> + const devBuildId = 'development'
>
> Won't this bring the mentioned issue #2014
> <#2014> back?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#3121 (review)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAL_Jpju2Pt__PkVrF1FRwTADOk6VLYLks5stPUSgaJpZM4P8Z1r>
> .
>
|
d2817ef
to
60ff231
Compare
60ff231
to
099b3f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great @kpdecker, just tried and seems to work pretty well. One thing I noticed is that hot updates that contain errors (throw etc) don't get sourcemaps client side. Example:
export default class extends React.Component {
render() {
if(typeof window !== 'undefined') {
throw new Error('Error')
}
return null
}
}
This reverts commit 964f229.
Thanks @kpdecker ❤️ |
This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread. |
Fixes source maps so they work properly when run in concat chunks, pages, or dynamic chunks.
Updates files names in dev mode to be constant so breakpoints will persist across page refreshes.
Fixes #2990