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

Compare the sizes of multiple packages and analyze package.json #441

Merged
merged 8 commits into from
Nov 17, 2019

Conversation

jonahsmith
Copy link
Contributor

@jonahsmith jonahsmith commented Nov 10, 2019

This PR adds package comparison using commas in the search bar and the ability to upload a package.json from the index page. Fixes #133

This is very "minimum viable implementation" — there's definitely room for improvement, especially on the frontend of the comparison page, but I wanted to put this out there as a possibility for the implementation of the core functionality. Happy to chat about and reconsider any of the design decisions here!

@vercel
Copy link

vercel bot commented Nov 10, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/styfle/packagephobia/b1pw5z8b5
🌍 Preview: https://packagephobia-git-fork-jonahsmith-master.styfle.now.sh

@styfle
Copy link
Owner

styfle commented Nov 10, 2019

Hi Jonah,

Thanks for the PR! I was hoping someone would implement this feature 😄

I'll need to spend some time reviewing this feature before merging but so far it looks great! 🥇

src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/pages/_document.ts Outdated Show resolved Hide resolved
src/util/constants.ts Outdated Show resolved Hide resolved
@jonahsmith
Copy link
Contributor Author

Awesome thanks! Just did a first pass on your initial comments, will come back for the remaining ones soon.

src/pages/parse-failure.tsx Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
src/server.ts Outdated Show resolved Hide resolved
let data: any[] = [];
req.on('data', chunk => data.push(chunk));
req.on('end', () => {
const [packageString] = data.toString().match(/{[\s\S]+}/) || [];
Copy link
Owner

@styfle styfle Nov 14, 2019

Choose a reason for hiding this comment

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

What's the purpose of this regex? Can you instead use JSON.parse(data.toString())?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the data comes through as multipart/form-data, which includes the content boundaries (https://www.w3.org/TR/html4/interact/forms.html#h-17.13.4.2). This seems like one lightweight way to parse the contents out, but definitely open to better options if you have any thoughts.

flexDirection: 'column',
alignItems: 'center',
justifyContent: 'center',
};
Copy link
Owner

Choose a reason for hiding this comment

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

I think you can use import PageContainer from '../components/PageContainer'; here from PR #450

Copy link
Owner

Choose a reason for hiding this comment

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

Actually, now that I think about it. We don't need a separate page for parse failures. We need to modify the 500 error page to print a user friendly error.

Perhaps create something like class PageError extends Error that we can then detect in the router to determine if the error should be rendered to the user.

src/pages/index.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@styfle styfle left a comment

Choose a reason for hiding this comment

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

This is wonderful, great job! 🏅

Thanks for taking the time to make this PR! 🤩

@styfle styfle merged commit 0101011 into styfle:master Nov 17, 2019
@jonahsmith
Copy link
Contributor Author

Thanks so much for your help and patience with this!

@mesqueeb
Copy link

Is this already usable on the front-end of bundlephobia?

@styfle
Copy link
Owner

styfle commented Jan 15, 2020

Yes

@mesqueeb
Copy link

@styfle It's strange, it the UI on the main website doesn't indicate we can compare packages at all:
https://bundlephobia.com

Even if I type something like: marked, markdown-it.
https://bundlephobia.com/result?p=marked,%20markdown-it

Also, when tapping enter on any auto-completed package name, it gives the page for just that package, loosing any other packages I had type before that with comma's.

Is there anything I can perhaps help with to improve this UI? I'm a Vue developer I can help.

@styfle
Copy link
Owner

styfle commented Jan 18, 2020

@mesqueeb I think you're confused. This repo is Package Phobia, not Bundle Phobia.

Here's the working page: https://packagephobia.now.sh/result?p=marked,markdown-it

Read more about the similar tools in the readme here: https://github.com/styfle/packagephobia/blob/master/README.md#how-is-this-different

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.

Compare the size of multiple packages
3 participants