-
Notifications
You must be signed in to change notification settings - Fork 14
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: Unable to shorten URL #696
base: master
Are you sure you want to change the base?
Conversation
Tests passed but got this coveralls issue. |
@@ -143,7 +143,14 @@ function createServerlessApp() { | |||
'/results', | |||
asyncMiddleware(async (req, res) => { | |||
let { d } = req.query | |||
const unpacked = jsonpack.unpack(d) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we would like to keep both implementations here in order to fallback to jsonpack
if the data isn't compressed so that older links are still accessible, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would probably be ideal, yes. I'll have to find some time to get back to this but some possibilities might be:
- Try to detect if the data has been compressed with
lz-string
in the handler here, although I don't see provided function fromlz-string
to do this. - Create a new major version with this breaking change.
- Add a new request param in the CLI/Node app here that indicates compression with
lz-string
, then check for the presence of that request param in this handler.
My preference would be for the last option.
I'm currently unable to access my report because of the url not shortening. Any progress on this? jherr uses bundlewatch in his new ProNextJS course, by the way -- that's how I discovered it. (recent uptick in downloads) |
Replaces
jsonpack
withlz-string
to help address #134.Corresponding bundlewatch PR is bundlewatch/bundlewatch#436.