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

Move jest to dev deps #97

Closed
wants to merge 1 commit into from
Closed

Move jest to dev deps #97

wants to merge 1 commit into from

Conversation

dawsbot
Copy link

@dawsbot dawsbot commented Feb 19, 2020

Closes #96

Tested locally, all works as expected!

@jhnns
Copy link
Member

jhnns commented Mar 9, 2020

Thanks for the PR ❤️

Jest is executed as postinstall script (see here and here) because we need to verify that the downloaded public suffix list is working as expected. Hence it's a regular dependency. Jest itself is not used during runtime, it's only executed after your npm install.

See also #91 #93

We will change this with the next version which is a complete rewrite in TypeScript. In the next version, people can decide if they want to update the internal list or not.

@jhnns jhnns closed this Mar 9, 2020
@dawsbot
Copy link
Author

dawsbot commented Mar 9, 2020

@jhnns Thanks for the information here, I didn't realize that's how this works.

On a separate note, what are your thoughts about having a lite version of this package? We're finding it adds quite a bit to our bundle sizes, but we need to be more performant. We're considering rewriting a lite version of this package that takes a few short-cuts.

Do you have any advice or warnings before we go for that @peerigon ?

@jhnns
Copy link
Member

jhnns commented Mar 18, 2020

I know, the package is still quite big. But I haven't found any possibility for shortcuts. What shortcuts do you have in mind?

@dawsbot
Copy link
Author

dawsbot commented Mar 18, 2020

@jhnns thanks for being open to this suggestion.

I think there are some great possibilities by reducing the size of the included json files used for parsing. Perhaps there are some regex approaches that can replace the heavy json files?

I just jumped into the codebase to give more specific examples but found the build system to be difficult to reason about. It sounds like this is a great feature so that the various json files are fetched up-to-date, but alas I'm stumped. Here are some heavy json files, sorted by size:

image

EDIT: We use this package on our lite PWA and find it's really slowing us down. I'm happy to help contribute and even maintain this if you have any ideas how I might be able to help here @jhnns 🙌 Thanks for a great package

@jhnns
Copy link
Member

jhnns commented Mar 23, 2021

@dawsbot I just saw your comment a year ago :)

The big JSON files you found are the smallest serialization format I could come up for the public_suffix_list. It's a serialized trie data structure with a custom format to encode it. The format is described here and here. The current bundled package size is around 70kb. There's still some repetition of words in the file, but imo that's the job for DEFLATE-like compression algorithms like gzip.

I don't think that it can be reduced significantly. Version 1 of this package used a big regex like you suggested and file size was way worse (around 200 kb). The regex was so big that it almost crashed IE11 😁

But I'm open for better/smaller solutions :)

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.

Remove jest from normal installs
2 participants