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

Solving : Fix required not defined #169

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

linediconsine
Copy link

@linediconsine linediconsine commented Jan 4, 2024

Hi, This patch aim to solve #170

the actual code rely on required be defined in the browser but this is not the case in my tests on Chrome browser:

Actual code

(typeof require?.resolve === 'function'
			? require.resolve('axe-core/axe.min.js')
			: 'node_modules/axe-core/axe.min.js');

console log

VM87:1 Uncaught ReferenceError: require is not defined
    at <anonymous>:1:1
(anonymous) @ VM87:1

Screenshot on Chrome
error

Patch proposed :

(typeof require != 'undefined' && typeof require?.resolve === 'function'
			? require.resolve('axe-core/axe.min.js')
			: 'node_modules/axe-core/axe.min.js');

console log

'node_modules/axe-core/axe.min.js'

after patch

tagging last active developers @MehmetYararVX @johnhwhite for a review on this if possible

@linediconsine
Copy link
Author

Let me know any feedback on this

thanks

@linediconsine linediconsine changed the title Solving : Fix required not defined Solving : Fix required not defined https://github.com/component-driven/cypress-axe/issues/170 Jan 5, 2024
@linediconsine linediconsine changed the title Solving : Fix required not defined https://github.com/component-driven/cypress-axe/issues/170 Solving : Fix required not defined #170 Jan 5, 2024
@linediconsine linediconsine changed the title Solving : Fix required not defined #170 Solving : Fix required not defined Jan 5, 2024
@linediconsine
Copy link
Author

linediconsine commented Jan 8, 2024

On local nom run test give me

 
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  test.cy.js                               699ms        1        1        -        -        - │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✔  All specs passed!                        699ms        1        1        -        -        -  

I see a timeout error on nom run test logs in pipeline testing, is it there a way to re-test?

@courentin
Copy link

@linediconsine thanks for your fix! Maybe you can push an empty commit to trigger the CI or create another PR?

@bmordue any chance to merge this one-liner and trigger a new release?
This would help us :)

Thanks a lot!

src/index.ts Outdated Show resolved Hide resolved
Co-authored-by: Corentin Hembise <[email protected]>
@adamnorbacker
Copy link

Any news on this PR? Also have this issue in our project.

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.

4 participants