CI: https://ci.nodejs.org/job/node-test-pull-request/1700/

@mscdex can you shed some light on what specifically is not supported to cause the deopt for using const?

@evanlucas I'm not enough of a v8 guru to understand. At first I thought it had to do with using a ternary expression in the value for const variables, but I've seen it happen when assigning without ternary expressions too. shrug. The actual aborted optimization reason every time though is "Unsupported phi use of const variable."

Thanks

CI is all green again.

LGTM

/cc @Trott @mhdawson

LGTM. Maybe add Fixes: https://github.com/nodejs/node/issues/5299 in the metadata for the commit message.

Also, since this fixes CI, I'd be all for landing it in an expedited fashion.

LGTM

@Trott agreed we want to land to fix the CI. Asked James to review since he was one of the original reviewers and since we have his LGTM now I'd say we're ready to land

@Trott do you have cycles to land ? If not I'll try to fit it in this afternoon.

Landed in 7d1d3a6

this does not land cleanly on LTS, would someone be willing to backport it?

@thealphanerd I'm guessing it isn't landing cleanly because #4892 has not landed yet. So if that can't land right away, maybe this can also be delayed until that lands first?

/cc @mscdex in case he wants to go ahead and backport. I'm not sure whether there's a reason to push this ahead of that other PR, though.

@Trott good eye... this will land with #4892

@Trott The only thing to be backported from this PR if the original url perf commit isn't also backported, are merely the few changes made to url.resolveObject() (changing const to var).

I'm moving the label to don't land for right now. We can change it back if #4892 ever lands