search

flackr / scroll-timeline

A polyfill of ScrollTimeline.
Apache License 2.0
891 stars 84 forks source link

Avoid compressing constructor names when running tests #200

Closed johannesodland closed 5 months ago

johannesodland commented 6 months ago

Adding this PR for discussion.

Microbundle compresses the build with terser by default. This results in CSSOM constructor names being replaced with t or similarly compressed names, which causes some subtests to fail.

Subtests using the css-typed-om test-helper.js fail, as the test-helper uses the constructor name to determine how to compare arguments.

I don't think microbundle allows for configuring terser directly. Configuration seems to be limited to enabling or disabling compression. The only available option I can see is building an uncompressed bundle for testing. Otherwise we would need to configure rollup, babel and terser ourself to ensure terser doesn't compress constructor names.

bramus commented 5 months ago

Feels a bit weird to run the tests with a different build, as there can be subtle differences that work / don’t work in the regular type of build. Ideally, the build used in tests is also the one that ships to users.

Or won’t this change affect regular users?

johannesodland commented 5 months ago

Feels a bit weird to run the tests with a different build, as there can be subtle differences that work / don’t work in the regular type of build. Ideally, the build used in tests is also the one that ships to users.

Or won’t this change affect regular users?

I agree.

Preferably the constructor names shouldn't be compressed, as users should also be able to check the constructor name.

As the build is now some tests are failing due to the compression, and as long as these tests are failing we won't be able to notice any regression on the functionality that is under test, such as view-timeline-get-set-range.html.

For the long term I think we need more control over the build. Maybe we need a different build setup.

Until then it's a choice between testing the same build as we ship while loosing out on some tests, or having better test coverage and the ability to discover regressions.

johannesodland commented 5 months ago

Closing as this is addressed in https://github.com/flackr/scroll-timeline/issues/219, #224 and following PRs