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

A11y error: The accessibility scan encountered an error. {} #12622

Closed
HosseinRashno opened this issue Sep 30, 2020 · 24 comments
Closed

A11y error: The accessibility scan encountered an error. {} #12622

HosseinRashno opened this issue Sep 30, 2020 · 24 comments

Comments

@HosseinRashno
Copy link

Describe the bug
I've followed this tutorial to add "storybook-addon-a11y" to my project. Now it shows "Accessibility" tab in the addons tabs. It works well for most of the stories, but it shows an error for some of them. This is the error:

The accessibility scan encountered an error.
{}

The error object is empty so I have no idea what's the problem here. I've tried to add a console log in this file to see the error in the console, and the result was this:
TypeError {}

To Reproduce
I've added the a11y addon to my project and the error happened!

Expected behavior
It should not throw an error for no reason, if something is going wrong, it should show a clear error.

Screenshots
image

System

  System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
  Binaries:
    Node: 12.18.3 - /usr/local/bin/node
    Yarn: 1.22.4 - /usr/local/bin/yarn
    npm: 6.14.6 - /usr/local/bin/npm
  Browsers:
    Chrome: 85.0.4183.121
    Firefox: 79.0
    Safari: 13.1.2
  npmPackages:
    @storybook/addon-a11y: ^6.0.22 => 6.0.22
    @storybook/addon-actions: ^6.0.20 => 6.0.20
    @storybook/addon-essentials: ^6.0.20 => 6.0.20
    @storybook/addon-links: ^6.0.20 => 6.0.20
    @storybook/html: ^6.0.20 => 6.0.20
@HosseinRashno
Copy link
Author

I've found the source of the error but still, I can't understand why this code causes an error.
So I use lit-element to create a web-component. This is the properties method of the lit-element class component:

  static get properties () {
    return {
      href: { type: String },
      type: { type: String },
      attributes: { type: Object }
    }
  }

As soon as I remove or rename attributes property, the error goes away. I believe this error is related to "storybook" and not "lit-element". I'll keep this issue open for a while to see if anyone knows what's going on here.

@shilman shilman added addon: a11y maintenance User-facing maintenance tasks P3 help wanted labels Oct 1, 2020
@shilman
Copy link
Member

shilman commented Oct 1, 2020

I agree, we should probably give a better error message here. Any takers?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 1, 2020

Automention: Hey @Armanio @CodeByAlex @jsomsanith, you've been tagged! Can you give a hand here?

@imshubhamsingh
Copy link

@shilman I would want to give it a try. I've been using storybook for quite some time. I would also be good for me to know how storybook works under the hood and give something back to it.

@shilman
Copy link
Member

shilman commented Oct 6, 2020

@imshubhamsingh Thank you for wanting to make Storybook better! 🙏 Please check out CONTRIBUTING.md to get started and feel free to ask questions in our Discord.

@imshubhamsingh
Copy link

@shilman I'll debug this and hopefully share information soon on this. Thanks for info ✌️.

@imshubhamsingh
Copy link

imshubhamsingh commented Oct 11, 2020

@HosseinRashno can you share a repo which reproduces this error. I created storybook for lit-element but didn't face such issue.

@stale
Copy link

stale bot commented Dec 25, 2020

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Dec 25, 2020
@jsomsanith
Copy link
Contributor

jsomsanith commented Apr 17, 2022

Hi 👋
Is this still a bug ? If so, can anyone help me reproduce it ? (A repo I can clone would be cool)

@stale stale bot removed the inactive label Apr 17, 2022
@Stunext
Copy link

Stunext commented Apr 18, 2022

With Vite builder:

Didn't work on 6.4.18 with old repository "storybook-builder-vite" (Build error)

It worked with the update to "@storybook/builder-vite" and version 6.4.19

Again it stopped working with version 6.4.20 and higher. No compilation errors; but it shows the aforementioned error:

The accessibility scan encountered an error. {}

@jsomsanith https://github.com/Stunext/a11y-bug

@jsomsanith
Copy link
Contributor

jsomsanith commented Apr 18, 2022

Hey @Stunext thank you for your a11y-bug repo.
I don't know vite, but from what Ii see is that webpack and vite don't seem to bundle the axe-core commonJS the same way.
In the a11y runner, we first dynamic-import axe-core, then call axe.run on the root element of the story.

  • webpack: import('axe-core') ends up with a module containing { default: { run, reset, ...}, run, reset, ...}
  • vite: import('axe-core') ends up with a module containing { default: { run, reset, ...}}

The code in axe-core is something like

const axe = {};
// a bunch of code adding functions in axe object

module.exports = axe;

With both bundlers we have the default in the dynamic import module containing all the functions, but with webpack, we also have access to the functions directly. This makes sense as, in theory, we can statically import default containing all the functions, of destructure it to import the named functions.
Don't know why vite ends up with only the default export.

But I'll provide a fix to be compatible with both, as both have access to the module default.

@IanVS
Copy link
Member

IanVS commented Apr 22, 2022

I believe the difference boils down to how vite vs webpack treat compatibility between commonjs and esm. Technically, according to the spec, import() is similar to import * from, in the sense that default exports are available on the default property. Webpack (and rollup, which vite uses in production), also seem to check to see if the module being imported was a cjs file initially, and if so, they plop more properties on the results of the import().

All that said, I think the most correct thing to do here, would be to use the module default. I needed to do something very similar for storybook react: #17987

IanVS added a commit that referenced this issue Apr 22, 2022
Issue: #12622

`Axe-core` exposes a commonjs module equivalent to 
```javascript
const axe = {};
// a bunch of code adding functions in axe object

module.exports = axe;
```
Storybook with builder-vite wraps axe-core dynamic import and ends up with a module containing only default export.
```javascript
// axe-core module with vite
{
  default: { run, reset, ... }
}
```

Webpack has another behavior, and ends up with a module containing default and all the named export function.
```javascript
// axe-core module with vite
{
  default: { run, reset, ... },
  run,
  reset,
  ...
}
```

But `addon-a11y` imports `axe-core` and use it directly with `axe.run()` for example, consuming the named exports.
For storybook with vite, it fails as `axe.run()` doesn't exist, whereas `axe.default.run()` exists.

## What I did

In `addon-a11y`, consume axe default export as it is always available.

## How to test

- [ ] Is this testable with Jest or Chromatic screenshots?
- [ ] Does this need a new example in the kitchen sink apps?
- [ ] Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

<!--

Everybody: Please submit all PRs to the `next` branch unless they are specific to the current release. Storybook maintainers cherry-pick bug and documentation fixes into the `master` branch as part of the release process, so you shouldn't need to worry about this. For additional guidance: https://storybook.js.org/docs/react/contribute/how-to-contribute

Maintainers: Please tag your pull request with at least one of the following:
`["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]`

-->
@jsomsanith
Copy link
Contributor

@IanVS Thanks for the explanation and the PR review :)

@joshrobertsv2
Copy link

@jsomsanith has this fix been released? if so, which release? I am experiencing this issue after switching over to storybook for vite. Any input greatly appreciated!

@IanVS
Copy link
Member

IanVS commented May 27, 2022

@joshrobertsv2 it was first included in v6.5.0-beta.0. If you're having an issue after that version, feel free to open an issue on the vite-builder repo, ideally with a reproduction, and we can take a look.

@joshrobertsv2
Copy link

Thanks @IanVS !

@taitems
Copy link

taitems commented Oct 28, 2022

FYI for anyone else who ends up here, I was receiving this on Storybook 7.0.0-alpha.40 for svelte-vite, because a default install of this plugin goes to 6.5.x. Matching the versions made it go away.

-"@storybook/addon-a11y": "^6.5.13",
+"@storybook/addon-a11y": "^7.0.0-alpha.40",
"@storybook/addon-essentials": "^7.0.0-alpha.40",
"@storybook/addon-interactions": "^7.0.0-alpha.40",

@IanVS
Copy link
Member

IanVS commented Oct 28, 2022

@taitems note that you can also use npx sb@next add @storybook/addon-a11y to install an addon, which will use the right version, and will add it to your .storybook/main.js for you.

@weo3dev
Copy link

weo3dev commented Mar 20, 2023

@IanVS - Is your helpful note still applicable with current and RC, and in future, v7 ? I would find it immensely helpful if a knowledgeable tidbit like this was surfaced a bit more clearly in the main documentation, as I have ended up here specifically, multiple times. Also, it is quite possible I should either decrease or increase intake of caffeine. :: tossup :: 😵‍💫

@IanVS
Copy link
Member

IanVS commented Mar 20, 2023

@weo3dev what are you referring to? As far as I know, npx storybook add has not been changed for 7.0. @jonniebigodes I wonder if it should be mentioned on https://storybook.js.org/docs/7.0/react/addons/install-addons.

@weo3dev
Copy link

weo3dev commented Mar 20, 2023

Oh - apologies for not being clear - using the specific callout of 'sb@next' is really great, and works! Was just wondering if that is a hook, for lack of a better term in my brain, that we will be able to use moving forward to keep add ons up to date more easily.

@jonniebigodes
Copy link
Contributor

jonniebigodes commented Mar 20, 2023

@IanVS I'm aware of this, and this is something that I'm going to bring over to the team (Storybook & DX) so that we provide a good experience for it. As most addons snippets mention only the package managers (e.g., yarn, npm, pnpm) and we probably should start pushing more into using the add command the way to go and potentially avoid similar situations. Thanks for putting this on my radar 🙏

@IanVS
Copy link
Member

IanVS commented Mar 20, 2023

@weo3dev you can continue to use it, but once 7.0 becomes "stable", then @next will start to refer to 8.0 alpha versions. This is an npm tag, and you can see what version it corresponds to here: https://www.npmjs.com/package/storybook?activeTab=versions

@weo3dev
Copy link

weo3dev commented Mar 20, 2023

Thank you so much for the clarification! Y'all rock. Thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants