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

Function uniqueId always returns a 12 chars id #930

Closed
wants to merge 1 commit into from
Closed

Function uniqueId always returns a 12 chars id #930

wants to merge 1 commit into from

Conversation

alxscms
Copy link

@alxscms alxscms commented May 29, 2018

The function uniqueId was making this library "impure". The output of the minifier could be different from one call to another using the same input and using the option maxLineLength.

Let me explain:

The function joinResultSegments is relying on the segments length to decide whether to add a newline separator or not when joining them while using the option maxLineLength. As the function uniqueId was generating a random length ID, sometimes you could have different outputs with HTML elements being moved to a new line while it wasn't on a previous call.

I changed the code of uniqueId a bit so that the returned ID is always a 12 chars string.

@XhmikosR XhmikosR requested a review from alexlamsl July 7, 2018 21:00
Copy link
Collaborator

@alexlamsl alexlamsl left a comment

Choose a reason for hiding this comment

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

Please provide a test case (which may not fail all the time due to the random nature of length returned by uniqueId()) before we proceed.

For instance, minify('<div><?foo?></div>', { maxLineLength: 5 }) does not yield any difference with or without this change over a million runs.

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.

2 participants