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

Allow users to use any version of axe core #65

Closed
aldosuwandi opened this issue Oct 13, 2020 · 7 comments · Fixed by #69
Closed

Allow users to use any version of axe core #65

aldosuwandi opened this issue Oct 13, 2020 · 7 comments · Fixed by #69
Labels
bug Something isn't working released

Comments

@aldosuwandi
Copy link

aldosuwandi commented Oct 13, 2020

I see a couple of issues opened to upgrade axe core version, which is bundled in this library. (#36, #58)

To avoid this type of request in the future, I think we can make this awesome library more flexible by allowing users to use any axe core version.

I'm proposing to add a parameter to the injectAxe method, so users can specify their axe core source/version to overwrite the default version.

Sample scenario: Assuming the user wants to use the latest axe core

  1. Install the latest axe-core
npm install -D axe-core@latest
  1. Load axe.min.js and pass it into the injectAxe method
cy.readFile('node_modules/axe-core/axe.min.js', 'utf-8').then((content) => {
    cy.injectAxe(content);
    cy.checkA11y();
})
@cshouts-tasc
Copy link
Contributor

It seems like npm has already solved this problem. Make axe-core a peer dependency and everything will work as intended with the developers preferred version of axe-core without any additional setup on their end.

Current consumers of the package should be able to continue using cy.injectAxe() and cy.configureAxe() in the same way they do today.

@aldosuwandi
Copy link
Author

aldosuwandi commented Oct 27, 2020

Currently, axe-core is bundled within this library, so unless we change the build setup, the peer dependency approach won't work.
see: #37 (comment)

The proposed solution should work if we decide to keep the bundled approach. But, if we want to unbundle it; then, peer dependency should be enough.

@sapegin
Copy link
Member

sapegin commented Oct 27, 2020

Now I see the problem! It's bundling all dependencies, which doesn't sound like an optimal solution:

"build": "parcel build src/index.js --bundle-node-modules",

And looks like there were reasons to bundle it though I'm not sure what exactly was the problem:

#37 (comment)

@sapegin sapegin added the bug Something isn't working label Oct 27, 2020
@sapegin sapegin pinned this issue Oct 27, 2020
@sapegin
Copy link
Member

sapegin commented Oct 27, 2020

I've done some experiments on my project that uses cypress-axe.

When I try to use not bundled code, it fails with “fs.readFileSync is not a function”:

image

However, using axe.source as suggested in #38 works fine.

I suggest we make a version with axe.source without bundling and see what other problems folks may have.

sapegin added a commit that referenced this issue Oct 27, 2020
This allows consumers to use their own version of axe-core and significantly reduces the installation size. Also allow any 3.x or 4.x versions of axe-core in peer dependencies.

Closes #65, closes #59
@sapegin
Copy link
Member

sapegin commented Oct 27, 2020

I've opened a pull request #69 — this works perfectly in my project. Would be great if anyone else could test it too.

@aldosuwandi
Copy link
Author

Thanks, @sapegin. I'm not sure either why @avanslaars decided to bundle it. If we can unbundle it, that will be great.
I tried PR #69 in my project and found an issue. Please see my comment here.

sapegin added a commit that referenced this issue Oct 27, 2020
This allows consumers to use their own version of axe-core and significantly reduces the installation size. Also allow any 3.x or 4.x versions of axe-core in peer dependencies.

Closes #65, closes #59
@github-actions
Copy link

🎉 This issue has been resolved in version 0.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sapegin sapegin unpinned this issue Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants