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] revert #2354 and fix double character decoding a different way #2435

Merged
merged 6 commits into from
Sep 17, 2021

Conversation

benmccann
Copy link
Member

Fixes #2401

Reverts most of #2354, but leaves the test for #1746

@changeset-bot
Copy link

changeset-bot bot commented Sep 15, 2021

🦋 Changeset detected

Latest commit: ce76e03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -231,15 +231,31 @@ function get_params(array) {
// src/routes/[x]/[y]/[z]/svelte, create a function
// that turns a RegExpExecArray into ({ x, y, z })

// input has already been decoded by decodeURI
// now handle the rest that decodeURIComponent would do
const d = /** @param {string} s */ (s) =>
Copy link
Member

Choose a reason for hiding this comment

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

This looks the same like in build - maybe put it into a common file for both to use? utils/encoding.js or something like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it is the same, but I'm not sure it's that easy to share code between the two because the one in build is a string that gets written out to a file and isn't normal code. I think I could still do it, but would probably need to setup a new file to be output in the Rollup config as well. I'm just not sure it's worth it at the moment given all the other stuff there is to do though I do agree it'd be nicer

@benmccann
Copy link
Member Author

I'm going to go ahead and merge this since we just got another report of it in #2451

@benmccann benmccann merged commit 9631200 into master Sep 17, 2021
@benmccann benmccann deleted the issue-2401 branch September 17, 2021 17:37
sidharthv96 added a commit to sidharthv96/kit that referenced this pull request Sep 19, 2021
* 'master' of https://github.com/sveltejs/kit: (322 commits)
  Version Packages (next) (sveltejs#2438)
  [fix] revert sveltejs#2354 and fix double character decoding a different way (sveltejs#2435)
  chore: update dependencies (sveltejs#2447)
  [docs] add link to envPrefix option in env var FAQ (sveltejs#2445)
  Fix invalid changeset. Thanks GitHub bot :-p
  [feat] use the Vite server options in dev mode (sveltejs#2232)
  [fix] provide valid value for `letter-spacing` CSS property in template (sveltejs#2437)
  [docs] fix typo (sveltejs#2443)
  [fix] add svelte field when packaging (sveltejs#2431)
  Version Packages (next) (sveltejs#2428)
  [chore] update lockfile
  [fix] update to TS 4.4.3 (sveltejs#2432)
  [chore] add links to repository and homepage to package.json (sveltejs#2425)
  docs: use fragment for prefetching link (sveltejs#2429)
  [fix] encodeURI during prerender (sveltejs#2427)
  Version Packages (next) (sveltejs#2420)
  revert change to versioning during release workflow
  chore: update vite-plugin-svelte (sveltejs#2423)
  [chore] expose Body generic to hook functions (sveltejs#2413)
  [feat] adapter-node entryPoint option (sveltejs#2414)
  ...
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.

@ not working in route paths
2 participants