Revert broccoli asset rev change

[apellerano-pw]

Fixes #275.

ember-cli-terser 4.0.2 contains a regression documented in #275 which forces people using both ember-cli-terser and broccoli-asset-rev to pin ember-cli-terser to 4.0.1.

One way forward is to revert the change which introduced the regression (PR #266) and release this as 4.0.3.

[st-h]

🙏 Please, please merge this and find a proper fix for https://github.com/ember-cli/ember-cli-terser/issues/265. It took me several days to figure out why source maps are broken and that it is caused by a regression with ember-cli-terser

[NullVoxPopuli]

is there any way you could add a test for this so the regression doesn't re-introduce itself?

Though, I guess because https://github.com/ember-cli/ember-cli-terser/pull/266 didn't contain any tests, we can't be sure that it was actually fixing anything either.

I'm in favor of merging this PR and releasing 4.0.3, and we really need tests added to cover this stuff before any additional changes around addon load order are done.

[NullVoxPopuli]

cc @rwjblue and @kellyselden ? idk who to tag :sweat_smile:

[NullVoxPopuli]

@st-h can you describe what sort of sourcemaps you are expecting?

• hi-fi pre-babel sourcemaps?
• or just "good enough" pre-terser sourcemaps?

[st-h]

@NullVoxPopuli the problem is that source maps usually contain a sources entry that contains the different classes of the application like myApp/routes/application. After the changes made in https://github.com/ember-cli/ember-cli-terser/pull/266 the sources entry only contains one link to the minified js file like assets/myapp-<fingerprint>.js. So the source maps just link to the minified js, and therefor won't work.

[NullVoxPopuli]

is it feasible for you to just use the older version?

The thing is that #266 fixed issue #265, so to revert #266 will re-break #265.

[st-h]

@NullVoxPopuli Yes, both. Sure it is possible to just stick to the old version if one requires working source maps. However, I am quite puzzled why the author of https://github.com/ember-cli/ember-cli-terser/pull/266 did not run into that issue. Given that there are quite a few users that commented on https://github.com/ember-cli/ember-cli-terser/issues/275, I can't be the only one having those issues. I think reverting https://github.com/ember-cli/ember-cli-terser/pull/266 would be a good idea as other users very likely will run into the same issue and it is quite difficult to track down which addon in which version causes it. (As I already mentioned, it took me days of trial and error) We certainly should be looking for a fix for #265 that does not render source maps useless. But at this point I think keeping things the way they are and hoping users that are affected by this regression will figure things out one day or the other is the worst option that we have.

Also, I wonder what did you do to resolve the issue, as you have also been commenting on #275? Might be there is a workaround none of us are aware of, which also worked for the author of https://github.com/ember-cli/ember-cli-terser/pull/266 without knowing about it. There probably is some sort of configuration or something that does not trigger the regression.

Pretty much the question at this point is: Do we want accurate line numbers, while source maps do not work at all for some users, or do we want incorrect line numbers for some users while source maps work for all users? Ideally we would have both, but at the moment I don't know where to start to tackle this issue and I pretty much have no time available at all.

[st-h]

@NullVoxPopuli any updates? What was your workaround to fix this issue in your codebase?

[NullVoxPopuli]

I think at this point we just have two versions folks use depending on the problem the users are facing. Until someone understands and can debug and fix both problems at the same time, that's how it's gotta be for now (imo, of course). The goal is to move to embroider, and ember-cli-terser is not used under embroider, so an option is to do that migration (I think, I could be wrong).

The main problem here is that different people want different levels of fidelity in production. Ultimately though, everyone should be comfortable with reading the "good enough" output in production. Because even outside of ember, that's sometimes all you can ask for.

[yads]

If different people want different behaviour, perhaps this should be a config instead of forcing people to pin an older version?