Browser-laptop: Update aphrodite to 1.2.4
Test plan
https://github.com/brave/browser-laptop/issues/10234
- Make sure the issue reported on #10029 does not happen again
See https://github.com/Khan/aphrodite/blob/master/CHANGELOG.md for changelog
luixxiul
All 6 comments
Removing milestone and marking as reverted. This was removed with https://github.com/brave/browser-laptop/commit/38f46189571710189a3702926d8d40b465fc953a which was needed to fix https://github.com/brave/browser-laptop/issues/10029.
We should be able to consider upgrading again once https://github.com/brave/browser-laptop/issues/10133 is finished
Please note that per semver, the ^ means any compatible version. Having ^1.0.0 was already grabbing the latest 1.0 release. More info available at https://docs.npmjs.com/files/package.json
bsclifton
on 26 Jul 2017
@luixxiul actually that's not a bug with Aphrodite. It's not clear where it was added but Aphrodite checks for production env var and concatenates strings. It should be nothing for us be we still rely on legacy code that depends on className.
For example in main.js we are checking matches against classList like node.matches('[class^="popupWindow"]'). It works in dev but as soon as build is ready, there's no class starting with popupWindow, so we always return false for such checks in production.
The fastest take to unblock next release was to freeze Aphrodite before that addition. But short term is to eliminate all class-dependent code as they're fragile and just re-use Aphrodite at latest.
cezaraugusto
on 26 Jul 2017
@luixxiul if you upgrade, please be sure not to use the ^ in the version 馃槃 (ex: prefer "1.2.3" over "^1.2.3")
bsclifton
on 31 Jul 2017
Here's a discussion if another variable should be applied to minify classname: https://github.com/Khan/aphrodite/pull/246#issuecomment-319047097
luixxiul
on 1 Aug 2017
Removing milestone. We can attempt this once https://github.com/brave/browser-laptop/issues/10551 is completed
bsclifton
on 29 Aug 2017
aphrodite was updated to 1.2.4.
https://github.com/Khan/aphrodite/blob/master/CHANGELOG.md#124
luixxiul
on 5 Oct 2017
Related issues
eljuno
路
3Comments
bbondy
路
3Comments
briannyeko
路
3Comments
bsclifton
路
3Comments
antiroyalty
路
3Comments
Most helpful comment
@luixxiul actually that's not a bug with Aphrodite. It's not clear where it was added but Aphrodite checks for production env var and concatenates strings. It should be nothing for us be we still rely on legacy code that depends on className.
For example in main.js we are checking matches against
classListlikenode.matches('[class^="popupWindow"]'). It works in dev but as soon as build is ready, there's no class starting withpopupWindow, so we always return false for such checks in production.The fastest take to unblock next release was to freeze Aphrodite before that addition. But short term is to eliminate all class-dependent code as they're fragile and just re-use Aphrodite at latest.