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

replace <str>.length with Buffer.byteLength(<str>) #223

Merged
merged 2 commits into from Mar 2, 2019
Merged

Conversation

@hulkish
Copy link
Contributor

@hulkish hulkish commented Nov 15, 2018

Some chars represent a byte length greater than 1. This means string length could report incorrect parsedSize, for bundles - if any special chars are used. More on this here.

Some chars represent a byte length greater than 1. This means string length could report incorrect `statSize` if any special chars are used. More on this [here](https://sankhs.com/2016/03/17/content-length-http-headers/)
@EugeneHlushko
Copy link

@EugeneHlushko EugeneHlushko commented Nov 15, 2018

Looks legit, do you think you could add an auto test to cover this case?

@valscion
Copy link
Member

@valscion valscion commented Nov 16, 2018

Hi, thanks for this PR! @EugeneHlushko has a point — it would be nice if we could have a test to cover this case.

You can take a look into test/stats/with-module-concatenation-info/ how there's both a stats.json generated by webpack and the bundle.js it represents. The expected values webpack-bundle-analyzer would report are in expected-chart-data.js.

This is how the test is ran:

it('should support stats files with modules inside `chunks` array', async function () {
generateReportFrom('with-modules-in-chunks/stats.json');
const chartData = await getChartData();
expect(chartData).to.containSubset(
require('./stats/with-modules-in-chunks/expected-chart-data')
);
});

@hulkish
Copy link
Contributor Author

@hulkish hulkish commented Nov 16, 2018

@valscion can you point me in the right direction for this? Im trying to get an idea of how i am intended to generate the fixture for this test?

@valscion
Copy link
Member

@valscion valscion commented Nov 16, 2018

The best bet might be to create a small reproduction repository with plain webpack and generate a stats.json object and output bundle from there. Supply that output to the basis of your new test and test that the expected sizes match what you want.

@hulkish
Copy link
Contributor Author

@hulkish hulkish commented Nov 16, 2018

@valscion that makes sense, thank you.

I guess I was looking for a script that creates one for me, or something lol

@hulkish
Copy link
Contributor Author

@hulkish hulkish commented Nov 17, 2018

@valscion @EugeneHlushko will this do it?

th0r
th0r approved these changes Mar 2, 2019
@th0r th0r merged commit c0f0165 into master Mar 2, 2019
2 checks passed
@th0r th0r deleted the use-buffer-bytelength branch Mar 2, 2019
@th0r
Copy link
Collaborator

@th0r th0r commented Mar 2, 2019

@hulkish thanks, merged! Sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants