Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Fix query param handling #107

Merged

Conversation

lukastaegert
Copy link
Contributor

This will resolve rollup/rollupjs.org#118.

The issue is that URL.searchParams, which is an iterator, is traversed via a for-of loop. This, however, is transpiled under the assumption we are traversing an array (which of course fails). This PR does two things

  • replace the for-of loop with a more old-fashioned Array.forEach
  • does not rely on URL.searchParams which is only supported in rather new browsers (for instance not in IE11 which is still widely used in e.g. corporate environments) but uses plain old regular expressions to parse URL.search

I must admit I did not add a specific test yet (I'm not even sure this is possible for this kind of transpilation issue but I like to stand corrected), still I hope this gets merged soon. In the meantime I will try to directly reference the PR to make Rollup's REPL work again.

* not using a for-of loop on an iterator that is transpiled wrongly
* not using URL.searchParams which is only supported by rather new
  browsers
@Rich-Harris Rich-Harris merged commit 6ee092f into sveltejs:master Feb 3, 2018
@Rich-Harris
Copy link
Member

thank you — I'm not sure how to test this either. I guess given the browser support/transpiler thing it's more the consumer's responsibility, i.e. we should probably put some tests on rollupjs.org eventually!

@Rich-Harris
Copy link
Member

Released 0.6.0 with the fix

@lukastaegert
Copy link
Contributor Author

Thanks 👍
One thing to note: The transpilation was done by sapper, not by rollupjs.org, i.e. it was in the files distributed by sapper. So maybe it was actually the TypeScript compiler that did it. Might be worth taking a look if there are more places where iterators are used.

@lukastaegert lukastaegert deleted the fix-search-param-handling branch February 3, 2018 20:13
@lukastaegert
Copy link
Contributor Author

Rollupjs.org is now back to using the official version of sapper :)

@Rich-Harris
Copy link
Member

nice!

So maybe it was actually the TypeScript compiler that did it

Ah, gotcha. That makes sense.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REPL ignores "shareable" in URL but always loads default example
2 participants