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

fix: don't prepend publicPath with slash #4816

Merged
merged 1 commit into from
Nov 9, 2019

Conversation

haoqunjiang
Copy link
Member

fixes #3338
fixes #4184

Actually I don't know why the slash was added in the first place, seems
extraneous to me.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:

fixes vuejs#3338
fixes vuejs#4184

Actually I don't know why the slash was added in the first place, seems
extraneous to me.
@haoqunjiang haoqunjiang merged commit 6325793 into vuejs:dev Nov 9, 2019
@tjam0588
Copy link

tjam0588 commented Dec 6, 2019

Was this PR even reviewed? It broke our app. Thanks! smh

@haoqunjiang
Copy link
Member Author

How? Any reproductions?

@tjam0588
Copy link

tjam0588 commented Dec 7, 2019

How? Something was there. You removed it. Yes, the slash seemed extraneous, but I've got to imagine we aren't the only ones who had a process that took its existence into account. These are the kinds of changes that make people nervous about upgrading. Luckily, we were able to track it down and apply a fix.

@haoqunjiang
Copy link
Member Author

I'm very sorry for the inconvenience caused by this commit.

In theory, the old behavior is undocumented, unintended, and non-intuitive, so I didn't expect it to be depended on by any project.

And we've put this in the beta channel for 18 days before a stable release, so I really didn't intend it to break any production app. I'm deeply sorry for that.

Anyway, in retrospect,

  1. Such insignificant fixes should be included in a major release rather than a patch, considering every bug fix could be a breaking change to some users.
  2. I'm considering using the ~ version range for generated projects by default in the next major (or maybe minor, I haven't discussed with other team members yet) version of Vue CLI.
    Only fixes for regressions can be added to subsequent patch releases.
    Other bug fixes should be included in minor versions, and users need to run vue upgrade manually to upgrade.

What do you think?

@tjam0588
Copy link

tjam0588 commented Dec 9, 2019

Thankfully, it didn't break the production app. We caught it in our CI environment. It just caused a bit of panic as we scrambled to understand what had changed.

@qiulang
Copy link

qiulang commented May 18, 2020

We accidentally deleted package-lock.json all of sudden our web app stopped to work! We then finally found it was because we didn't add leading '/' our vue.config.js

I didn't update vue-cli, so which package was changed by this ?

My second question is what about the '/' in the end ? The document here https://cli.vuejs.org/config/#publicpath seems to say we need the '/' in the end. But thru our test we found we don't.

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