Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Update aphrodite to 1.2.4 #10062

Closed
luixxiul opened this issue Jul 19, 2017 · 6 comments
Closed

Update aphrodite to 1.2.4 #10062

luixxiul opened this issue Jul 19, 2017 · 6 comments

Comments

@luixxiul
Copy link
Contributor

luixxiul commented Jul 19, 2017

Test plan

#10234

  1. Make sure the issue reported on regression - Clicking inside Extension Popup Dismisses Popup #10029 does not happen again

See https://github.com/Khan/aphrodite/blob/master/CHANGELOG.md for changelog

@bsclifton
Copy link
Member

bsclifton commented Jul 25, 2017

Removing milestone and marking as reverted. This was removed with 38f4618 which was needed to fix #10029.

We should be able to consider upgrading again once #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 bsclifton removed this from the 0.21.x milestone Jul 25, 2017
@cezaraugusto
Copy link
Contributor

@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.

@bsclifton
Copy link
Member

@luixxiul if you upgrade, please be sure not to use the ^ in the version 😄 (ex: prefer "1.2.3" over "^1.2.3")

@luixxiul
Copy link
Contributor Author

luixxiul commented Aug 1, 2017

Here's a discussion if another variable should be applied to minify classname: Khan/aphrodite#246 (comment)

@luixxiul luixxiul changed the title Update aphrodite to ^1.2.3 Update aphrodite to 1.2.3 Aug 1, 2017
@luixxiul luixxiul added this to the 0.21.x (Nightly Channel) milestone Aug 1, 2017
@bsclifton bsclifton removed this from the 0.21.x (Nightly Channel) milestone Aug 29, 2017
@bsclifton
Copy link
Member

Removing milestone. We can attempt this once #10551 is completed

@luixxiul luixxiul changed the title Update aphrodite to 1.2.3 Update aphrodite to 1.2.4 Oct 5, 2017
@luixxiul
Copy link
Contributor Author

luixxiul commented Oct 5, 2017

aphrodite was updated to 1.2.4.

https://github.com/Khan/aphrodite/blob/master/CHANGELOG.md#124

@luixxiul luixxiul removed their assignment Nov 21, 2017
@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@bsclifton bsclifton removed this from the Triage Backlog milestone Oct 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants