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

fix(utils): make cache global instead of only setup in axe.run #1535

Merged
merged 4 commits into from
May 9, 2019

Conversation

straker
Copy link
Contributor

@straker straker commented May 1, 2019

Turns out that a lot of things can call axe.utils.getFlattenedTree without going through axe.run, including our own code when we cleanup plugins https://github.com/dequelabs/axe-core/blob/develop/lib/core/public/cleanup-plugins.js#L28. This caused the axe extensions to break.

Moving the creation of the nodeMap to the flatten tree code meant that the cache also had to exist at that point, which meant it was created in two different places (axe.run and getFlattenedTree). I decided it was cleaner to create a global private cache that always existed and could be used anywhere for any code that needed it. That way you weren't restricted to go through axe.run to have the cache available.

Linked issues: #1532

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Has documentation updated, a DU ticket, or requires no documentation change
  • Includes new tests, or was unnecessary
  • Code is reviewed for security by: Stephen

Copy link
Member

@stephenmathieson stephenmathieson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test demonstrating the bug this is preventing.

Obviously don't build an entire extension, but maybe a tiny/contrived "plugin" instead?

@straker
Copy link
Contributor Author

straker commented May 8, 2019

Good call. I created an integration test for plugins following our own docs code https://github.com/dequelabs/axe-core/blob/develop/doc/plugins.md. The last test breaks in the develop branch, as was expected.

@straker straker merged commit 91a04c5 into develop May 9, 2019
@straker straker deleted the cache branch May 9, 2019 16:38
stephenmathieson added a commit to mohanraj-r/axe-core that referenced this pull request May 10, 2019
* develop:
  feat(reporter): adds the rawEnv reporter which wraps raw and env data (dequelabs#1556)
  chore(i18n): Update Japanese locale (dequelabs#1549)
  fix(utils): make cache global instead of only setup in axe.run (dequelabs#1535)
  fix(aria-required-attr): don't require aria-valuemin/max (dequelabs#1529)
  docs: fixed small errors (dequelabs#1545)
  fix: check if property exists in cache of flattenedTree (dequelabs#1536)
  chore: update standard-version dep (dequelabs#1555)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants