-
Notifications
You must be signed in to change notification settings - Fork 85
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
feat: Don't bundle axe-core #69
Conversation
|
||
export const injectAxe = () => { | ||
cy.window({ log: false }).then((window) => { | ||
window.eval(axe); | ||
window.eval(axe.source); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using axe.source
will use the source from axe-core in cypress-axe's dependencies (not the main project). Thus it'll be locked to axe-core ^4.0.2.
To reproduce this issue, after installing cypress-axe, try installing the peer dependencies:
npm install --save-dev cypress axe-core@3.5.5
Once you run cy.injectAxe() and cy.checkA11y()
, you can see the injected axe-core version is 4.0.2 (not 3.5.5).
Rather than using axe.source
, I recommend using cy.readFile(), so the implementation will be:
export const injectAxe = () => {
cy.readFile('node_modules/axe-core/axe.min.js').then((content) => {
cy.window({ log: false }).then((window) => {
window.eval(content);
});
})
};
I tested this implementation, and it successfully changes the axe-core version to 3.5.5 (for the scenario above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no bundled axe-core anymore in pull request, it's coming from your project's dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I understand it's not bundled anymore.
Perhaps, I encountered this issue because I tested this change with npm link
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to copy the built file into my project's node_modules
. Not sure it's much better than linking ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably copy-pasting the built file is more suitable in this case. When I tested that way, this change works as expected 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, then I'm going to merge this and we could test the real thing and see what other issues we'll have ;-)
🎉 This PR is included in version 0.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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