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

Escaped params #146

Merged
merged 2 commits into from
Mar 22, 2018
Merged

Escaped params #146

merged 2 commits into from
Mar 22, 2018

Conversation

trotzig
Copy link
Contributor

@trotzig trotzig commented Mar 1, 2018

When urls are constructed in the Link component, all params are URI
encoded so that they won't break routing. When params are read however,
they are never decoded. This leads to an inconsistent behavior between
the server and the client when params are passed down the page component
as query. On the server, they come down untouched (encoded). On the
client, they are not encoded.

Consider this link:

  <Link route="foo" params={{ b: 'f/g' }}><a>Foo</a></Link>

and this page component

  FooPage.getInitialProps = async ({ query }) => {
    console.log(query.b);
  }

On the server, this page would log "f%2Fg" when navigated to. On the
client it would log "f/g".

By decoding params before they are injected into the query params, we
get the same behavior on the client and the server. We also make the
behavior consistent with how regular query params work.

trotzig added 2 commits March 1, 2018 14:06
In an attempt to fix fridays#145, I found that there was no test verifying the
current behavior of escaping params. This test should help prevent
future regressions.
Fixes fridays#145

When urls are constructed in the `Link` component, all params are URI
encoded so that they won't break routing. When params are read however,
they are never decoded. This leads to an inconsistent behavior between
the server and the client when params are passed down the page component
as `query`. On the server, they come down untouched (encoded). On the
client, they are not encoded.

Consider this link:

  <Link route="foo" params={{ b: 'f/g' }}><a>Foo</a></Link>

and this page component

  FooPage.getInitialProps = async ({ query }) => {
    console.log(query.b);
  }

On the server, this page would log "f%2Fg" when navigated to. On the
client it would log "f/g".

By decoding params before they are injected into the `query` params, we
get the same behavior on the client and the server. We also make the
behavior consistent with how regular query params work.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 96.137% when pulling 7f23584 on trotzig:escaped-params into 4630d19 on fridays:master.

@trotzig
Copy link
Contributor Author

trotzig commented Mar 5, 2018

Ping @fridays, not sure if you watch new PRs coming in.

@fridays
Copy link
Owner

fridays commented Mar 22, 2018

Thank you, great catch!

@fridays fridays merged commit 21437ea into fridays:master Mar 22, 2018
@trotzig trotzig deleted the escaped-params branch March 22, 2018 08:17
petertulala added a commit to petertulala/next-routes-with-locale that referenced this pull request Jan 19, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
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.

None yet

3 participants