Skip to content

Commit

Permalink
Revert encodeurl changes
Browse files Browse the repository at this point in the history
  • Loading branch information
pshrmn authored and mjackson committed Mar 26, 2019
1 parent 72e6832 commit 845d690
Show file tree
Hide file tree
Showing 12 changed files with 195 additions and 181 deletions.
24 changes: 12 additions & 12 deletions .size-snapshot.json
@@ -1,26 +1,26 @@
{
"esm/history.js": {
"bundled": 28234,
"minified": 12465,
"gzipped": 3635,
"bundled": 28364,
"minified": 12515,
"gzipped": 3683,
"treeshaked": {
"rollup": {
"code": 226,
"import_statements": 150
"code": 208,
"import_statements": 132
},
"webpack": {
"code": 1375
"code": 1324
}
}
},
"umd/history.js": {
"bundled": 34665,
"minified": 12190,
"gzipped": 4109
"bundled": 33174,
"minified": 12021,
"gzipped": 3992
},
"umd/history.min.js": {
"bundled": 32227,
"minified": 10251,
"gzipped": 3675
"bundled": 30690,
"minified": 10155,
"gzipped": 3616
}
}
7 changes: 0 additions & 7 deletions modules/LocationUtils.js
@@ -1,7 +1,5 @@
import resolvePathname from 'resolve-pathname';
import valueEqual from 'value-equal';
import encodeurl from 'encodeurl';
import tinywarning from 'tiny-warning';

import { parsePath } from './PathUtils';

Expand Down Expand Up @@ -52,11 +50,6 @@ export function createLocation(path, state, key, currentLocation) {
location.pathname = '/';
}
}

tinywarning(
location.pathname === encodeurl(location.pathname),
"location.pathname is not fully encoded, which may result in bugs."
);

return location;
}
Expand Down
12 changes: 9 additions & 3 deletions modules/__tests__/BrowserHistory-test.js
Expand Up @@ -57,9 +57,9 @@ describe('a browser history', () => {
});
});

describe('push with a non-encoded path string', () => {
it('logs a warning', done => {
TestSequences.WarnsForNonencodedPathname(history, done);
describe('push with a unicode path string', () => {
it('creates a location with decoded properties', done => {
TestSequences.PushUnicodeLocation(history, done);
});
});

Expand Down Expand Up @@ -87,6 +87,12 @@ describe('a browser history', () => {
});
});

describe('location created by encoded and unencoded pathname', () => {
it('produces the same location.pathname', done => {
TestSequences.LocationPathnameAlwaysSame(history, done);
});
});

describe('location created with encoded/unencoded reserved characters', () => {
it('produces different location objects', done => {
TestSequences.EncodedReservedCharacters(history, done);
Expand Down
12 changes: 9 additions & 3 deletions modules/__tests__/HashHistory-test.js
Expand Up @@ -60,9 +60,9 @@ describe('a hash history', () => {
});
});

describe('push with a non-encoded path string', () => {
it('logs a warning', done => {
TestSequences.WarnsForNonencodedPathname(history, done);
describe('push with a unicode path string', () => {
it('creates a location with decoded properties', done => {
TestSequences.PushUnicodeLocation(history, done);
});
});

Expand Down Expand Up @@ -90,6 +90,12 @@ describe('a hash history', () => {
});
});

describe('location created by encoded and unencoded pathname', () => {
it('produces the same location.pathname', done => {
TestSequences.LocationPathnameAlwaysSame(history, done);
});
});

describe('location created with encoded/unencoded reserved characters', () => {
it('produces different location objects', done => {
TestSequences.EncodedReservedCharacters(history, done);
Expand Down
20 changes: 0 additions & 20 deletions modules/__tests__/LocationUtils-test.js
Expand Up @@ -58,26 +58,6 @@ describe('createLocation', () => {
});
});
});

describe('pathname encoding', () => {
describe('with a non-encoded pathname', () => {
it('warns that it received an improperly encoded pathname', () => {
let consoleWarn = console.error; // eslint-disable-line no-console
let warningMessage;

// eslint-disable-next-line no-console
console.warn = message => {
warningMessage = message;
};

createLocation('/歴史');
expect(warningMessage).toBe('location.pathname is not fully encoded, which may result in bugs.');

// eslint-disable-next-line no-console
console.error = consoleWarn;
});
});
});

describe('with a path with no pathname', () => {
describe('given as a string', () => {
Expand Down
12 changes: 9 additions & 3 deletions modules/__tests__/MemoryHistory-test.js
Expand Up @@ -51,9 +51,9 @@ describe('a memory history', () => {
});
});

describe('push with a non-encoded path string', () => {
it('logs a warning', done => {
TestSequences.WarnsForNonencodedPathname(history, done);
describe('push with a unicode path string', () => {
it('creates a location with decoded properties', done => {
TestSequences.PushUnicodeLocation(history, done);
});
});

Expand Down Expand Up @@ -81,6 +81,12 @@ describe('a memory history', () => {
});
});

describe('location created by encoded and unencoded pathname', () => {
it('produces the same location.pathname', done => {
TestSequences.LocationPathnameAlwaysSame(history, done);
});
});

describe('location created with encoded/unencoded reserved characters', () => {
it('produces different location objects', done => {
TestSequences.EncodedReservedCharacters(history, done);
Expand Down
45 changes: 45 additions & 0 deletions modules/__tests__/TestSequences/LocationPathnameAlwaysSame.js
@@ -0,0 +1,45 @@
import expect from 'expect';

import execSteps from './execSteps';

export default function(history, done) {
const steps = [
() => {
// encoded string
const pathname = '/%E6%AD%B4%E5%8F%B2';
history.replace(pathname);
},
location => {
expect(location).toMatchObject({
pathname: '/%E6%AD%B4%E5%8F%B2'
});

// encoded object
const pathname = '/%E6%AD%B4%E5%8F%B2';
history.replace({ pathname });
},
location => {
expect(location).toMatchObject({
pathname: '/%E6%AD%B4%E5%8F%B2'
});
// unencoded string
const pathname = '/歴史';
history.replace(pathname);
},
location => {
expect(location).toMatchObject({
pathname: '/歴史'
});
// unencoded object
const pathname = '/歴史';
history.replace({ pathname });
},
location => {
expect(location).toMatchObject({
pathname: '/歴史'
});
}
];

execSteps(steps, history, done);
}
28 changes: 28 additions & 0 deletions modules/__tests__/TestSequences/PushUnicodeLocation.js
@@ -0,0 +1,28 @@
import expect from 'expect';

import execSteps from './execSteps';

export default function(history, done) {
const steps = [
location => {
expect(location).toMatchObject({
pathname: '/'
});

const pathname = '/歴史';
const search = '?キー=値';
const hash = '#ハッシュ';
history.push(pathname + search + hash);
},
(location, action) => {
expect(action).toBe('PUSH');
expect(location).toMatchObject({
pathname: '/歴史',
search: '?キー=値',
hash: '#ハッシュ'
});
}
];

execSteps(steps, history, done);
}
34 changes: 0 additions & 34 deletions modules/__tests__/TestSequences/WarnsForNonencodedPathname.js

This file was deleted.

5 changes: 4 additions & 1 deletion modules/__tests__/TestSequences/index.js
Expand Up @@ -20,6 +20,9 @@ export {
export { default as InitialLocationNoKey } from './InitialLocationNoKey';
export { default as InitialLocationHasKey } from './InitialLocationHasKey';
export { default as Listen } from './Listen';
export {
default as LocationPathnameAlwaysSame
} from './LocationPathnameAlwaysSame';
export { default as NoslashHashPathCoding } from './NoslashHashPathCoding';
export { default as PushEncodedLocation } from './PushEncodedLocation';
export { default as PushNewLocation } from './PushNewLocation';
Expand All @@ -29,6 +32,7 @@ export { default as PushSamePathWarning } from './PushSamePathWarning';
export { default as PushState } from './PushState';
export { default as PushStateWarning } from './PushStateWarning';
export { default as PushRelativePathname } from './PushRelativePathname';
export { default as PushUnicodeLocation } from './PushUnicodeLocation';
export { default as ReplaceNewLocation } from './ReplaceNewLocation';
export { default as ReplaceSamePath } from './ReplaceSamePath';
export { default as ReplaceState } from './ReplaceState';
Expand All @@ -37,5 +41,4 @@ export {
default as ReturnFalseTransitionHook
} from './ReturnFalseTransitionHook';
export { default as SlashHashPathCoding } from './SlashHashPathCoding';
export { default as WarnsForNonencodedPathname } from './WarnsForNonencodedPathname';
export { default as TransitionHookArgs } from './TransitionHookArgs';

1 comment on commit 845d690

@felixfbecker
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjackson Since this was merged 3 months ago, is there any ETA on when this will be released? We are affected by this bug sourcegraph/sourcegraph#4408

Please sign in to comment.