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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decrease file size #33

Merged
merged 29 commits into from May 30, 2018
Merged

Decrease file size #33

merged 29 commits into from May 30, 2018

Conversation

jhnns
Copy link
Member

@jhnns jhnns commented May 30, 2018

This PR introduces a trie-based approach instead of a big regular expression. The trie is built upon npm install using the postinstall hook. If something goes wrong, the postinstall script falls back to a list that has been compiled on prepublish.

Using the trie-based approach, I was able to reduce the parse size from 216 KB to 63 KB. The compressed size went down from 71 KB to 30 KB. And the best thing about it: it's even not a breaking change 馃榿.

@jhnns jhnns merged commit 59f951b into master May 30, 2018
@jhnns jhnns deleted the refactor/improve-file-size branch May 30, 2018 00:35
@vshulev
Copy link

vshulev commented Jun 1, 2018

@jhnns I believe this is causing installation to fail when running npm i -g parse-domain inside the Node Docker image. Tested this on images 8.9 and 8.11 and getting the same result:

+ npm i -g parse-domain

> parse-domain@2.1.1 postinstall /usr/local/lib/node_modules/parse-domain
> node scripts/build-tries.js

Downloading public suffix list from https://publicsuffix.org/list/public_suffix_list.dat... ok
Writing /usr/local/lib/node_modules/parse-domain/build/tries/current/icann.complete.json... 
Could not update list of known top-level domains for parse-domain because of EACCES: permission denied, mkdir '/usr/local/lib/node_modules/parse-domain/build/tries/current'
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! parse-domain@2.1.1 postinstall: `node scripts/build-tries.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the parse-domain@2.1.1 postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2018-06-01T14_32_15_916Z-debug.log

I was originally trying to run npm i -g semantic-release conventional-changelog-eslint - one of these two packages contains parse-domain as a dependency (not sure which one). However directly installing parse-domain produces the same error.

@jhnns
Copy link
Member Author

jhnns commented Jun 11, 2018

@vshulev thanks for letting me know 馃憤. Let's continue the discussion at #35

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.

None yet

2 participants