Skip to content

Conversation

@JounQin
Copy link
Contributor

@JounQin JounQin commented Jun 16, 2017

No description provided.

@posva
Copy link
Member

posva commented Jun 16, 2017

Could you add a test for it in the api.spec.js file please?

@JounQin
Copy link
Contributor Author

JounQin commented Jun 16, 2017

@posva Sorry but it should only work on IE9, and it seems there is no spec for fallback to hash mode before, so actually I have trouble about how to write the test spec.

@posva
Copy link
Member

posva commented Jun 16, 2017

I meant that you can test that the mode is actually forced to 'history' even when there's no browser support for pushState (https://github.com/vuejs/vue-router/blob/dev/src/util/push-state.js#L6)

@JounQin
Copy link
Contributor Author

JounQin commented Jun 16, 2017

But it will always fallback to abstract on server/Node.js and it should not be broken.

https://github.com/vuejs/vue-router/blob/dev/src/index.js#L49

Then I pretend it as browser inside, I don't know does it meet your requirement.

@posva
Copy link
Member

posva commented Jun 16, 2017

Sorry if I was too pushy there, I meant to have an open discussion about it 😅
Yes, it should default to abstract in Node. What you wrote looks pretty much like what I was thinking (without the dist files ofc, if you can remove that from the commit, that would be great 😄 )

@JounQin
Copy link
Contributor Author

JounQin commented Jun 16, 2017

But the spec indeed requires the dist files, because as I mentioned in comment: "we do not use Router top above directly because inBrowser has been initialized inside".

@yyx990803
Copy link
Member

The dist file will be overwritten between builds... this won't work. Let's just remove the spec.

@JounQin
Copy link
Contributor Author

JounQin commented Jun 17, 2017

@yyx990803 I've reset the last commit.

@yyx990803 yyx990803 merged commit 05a5f08 into vuejs:dev Jun 17, 2017
@JounQin JounQin deleted the fallback branch June 17, 2017 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants