We should skip the same URL

[xiaohulu]

Type: bug

The following has been addressed in the PR:

• There is a related issue
• All code has been formatted with prettier as per the readme code style guidelines
• Unit or Functional tests are included in the PR

Description:

I use Link widget to navigate, but If I click the same Link n times, I must click Back button n times to back to previous route. We should skip the same url.

[agubler]

@xiaohulu Thank you, do you mind adding a unit test for the change?

[codecov]

Codecov Report

Merging #317 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
- Coverage    97.2%   97.18%   -0.02%     
==========================================
  Files          99       99              
  Lines        5365     5367       +2     
  Branches     1202     1203       +1     
==========================================
+ Hits         5215     5216       +1     
- Misses        150      151       +1

Impacted Files	Coverage Δ
src/routing/history/StateHistory.ts	97.87% <100%> (-2.13%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 860910a...afb696f. Read the comment docs.

[codecov]

Codecov Report

Merging #317 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
+ Coverage    97.2%   97.25%   +0.05%     
==========================================
  Files          99      100       +1     
  Lines        5365     5397      +32     
  Branches     1202     1213      +11     
==========================================
+ Hits         5215     5249      +34     
+ Misses        150      148       -2

Impacted Files	Coverage Δ
src/routing/history/StateHistory.ts	100% <100%> (ø)	⬆️
src/shim/fetch.ts	0% <0%> (ø)	⬆️
src/shim/IntersectionObserver.ts	100% <0%> (ø)	⬆️
src/shim/WebAnimations.ts	100% <0%> (ø)	⬆️
src/shim/util/wrapper.ts	100% <0%> (ø)
src/has/has.ts	95.9% <0%> (+0.02%)	⬆️
src/widget-core/vdom.ts	98.65% <0%> (+0.04%)	⬆️
src/shim/ResizeObserver.ts	100% <0%> (+12.5%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 860910a...c3430d4. Read the comment docs.

[xiaohulu]

I do not know how to run StateHistory unit test. 😢

I write the unit test code here, I do not know if it can pass.

it('update path, skip the same path', () => {
	const history = new StateHistory({ onChange, window: sandbox.contentWindow! });
	history.set('foo');
	history.set('bar');
	history.set('bar');
	sandbox.contentWindow!.history.back();
	assert.equal(history.current, '/foo');
	assert.equal(sandbox.contentWindow!.location.pathname, '/foo');
});

[agubler]

@xiaohulu Hmmm that's interesting, I seem to be able to run the tests (on mac, node: v8.9.1 and npm: 6.4.1).

Have you tried, removing and reinstalling node_modules (and removing the package-lock too)?

[agubler]

@xiaohulu Do you think we should move the logic from _onChange that does a check against the current and previous value to set function?

const pathName = this._window.location.pathname.replace(/\/$/, '');
const value = stripBase(this._base, pathName + this._window.location.search);
if (this._current === value) {
	return;
}

[xiaohulu]

@agubler What is the base in the framework, and what is its purpose, and why this._current is assign to stripBase result

[agubler]

@xiaohulu Thanks!