salesforce/sa11y

Add ability to register a11y matcher in jest config once per project

mohanraj-r opened this issue · 5 comments

mohanraj-r commented

Motivation

The a11y matcher API has to be registered before its used. Currently this needs to be done in each test module as part of test setup as shown below.

import { registerA11yMatchers } from '@sa11y/jest';
beforeAll(() => {
    registerA11yMatchers();
});
describe('integration test @sa11y/jest', () => {
    it('should have a11y matchers working', () => {
        expect(expect.toBeAccessible).toBeDefined();
        expect(document).toBeAccessible();
    });
});

This could be made less tedious by registering the a11y matchers once in the jest config of the consuming project and then all the tests in the project can use the a11y API without any additional setup.

For example a consuming project should be able to register a11y matchers by extending the config as below in the jest.config.js at the project root:

const { jestConfig } = require('@sa11y/jest');

module.exports = {
    ...jestConfig,
    // Rest of the Project's Jest config ..
};

Resources

Similar setup can be found in

mohanraj-r commented

I am running into errors related to modules primarily because I think jest uses the CJS module system whereas the rest of the sa11y packages are using the ES modules. Not sure if it would help if I split the integration tests from the mono-repo into its own project.

In the integration tests package:

Baseline

  • Using the toBeAccessible() API after explicitly invoking the registerA11yMatchers() in the test module works as illustrated in this issue above.

extending jest.config.js directly with setup from sa11y

  • When I create a jest.config.js with
const { jestConfig } = require('@sa11y/jest');
module.exports = {
    ...jestConfig,
}

I run into the following error: SyntaxError: Unexpected token 'export'

$ /Users/mrajamanickam/src/sa11y/foundation/node_modules/.bin/jest --no-cache
/Users/mrajamanickam/src/sa11y/foundation/packages/jest/dist/index.js:8
export { toBeAccessible } from './matcher';
^^^^^^

SyntaxError: Unexpected token 'export'

The error means that jest is making use of the extended config, but is not able to understand the ES module syntax.

extending with local setup file

  • When I use setupFilesAfterEnv in jest.config.js instead
module.exports = {
    setupFilesAfterEnv: ['./jestSetup.js'],
}

with a local file jestSetup.js:

const { registerA11yMatchers } = require('@sa11y/jest');
registerA11yMatchers();

I get no errors when running jest - but the integration test fails because the toBeAccessible is not defined which probably means that the setup is not working. Also tried with

  • setupFilesAfterEnv: [require('./jestSetup.js')],
  • setupFilesAfterEnv: [require('<rootDir>/jestSetup.js')],
 ● integration test @sa11y/jest › should have a11y matchers working with setup in jest.config.js

   expect(received).toBeDefined()

   Received: undefined

     12 |     // registerA11yMatchers();
     13 |     it('should have a11y matchers working with setup in jest.config.js', () => {
   > 14 |         expect(expect.toBeAccessible).toBeDefined();
        |                                       ^
     15 |         // TODO(Fix) : Fails with TypeError: expect(...).toBeAccessible is not a function
     16 |         expect(document).toBeAccessible();
     17 |     });

Tried out many suggestions from

including

  • adding babel plugins to babel.config.js - didn't help
env: {
        test: {
            plugins: ['transform-es2015-modules-commonjs', 'dynamic-import-node', 'babel-plugin-transform-dynamic-import'],
        },
    },
  • adding "type": "module" to the @sa11y/jest package.json
    • got Error: Jest: Your version of Node does not support dynamic import - please enable it or use a .cjs file extension

@trevor-bliss If you have any suggestions please let me know.

trevor-bliss commented

Ok I pulled your branch and tried it out. I got it working after making a couple changes.

  1. Update dist files to be commonjs format

You were on to something when you mentioned the module format. You don't have to update any of your authored source code, but in the root level tsconfig.common.js file, you can change the module entry to commonjs so the dist files you produce are in a more compatible format.

  1. Fix packages/jest/package.json

The main and types entries are wrong here. Should point to dist/index.js instead of dist/jest.js.

  1. Simplify the Jest setup

This one is broader and more my opinion than a "bug". I think having @sally/jest export a jest config object is doing too much. Instead, it should export the registerA11yMatchers function and the consuming projects can call that function themselves in their Jest setup. sfdx-lwc-jest provides the full config because it's a full test runner. This is more of a "matchers" library being integrated into an existing Jest setup.

So what I would do is remove the registerA11yMatchers() invocation in setup.ts and update test-integ/jest.config.js to:

module.exports = {
    setupFilesAfterEnv: ['<rootDir>/jest-setup.js'],
};

where jest-setup.js is:

const { registerA11yMatchers } = require('@sa11y/jest');
registerA11yMatchers();

I think this is a reasonable thing for consuming projects to do so appropriate for this integration test package.

Finally, if you run the Jest tests from the top level this will still fail because that will use the root level jest config instead of the integration test package config. I'd either create a separate npm script for running tests in this package, or have each sub package extend a base config and then have this package also have the setupFilesAfterEnv entry to register the matchers.

mohanraj-r commented

That is awesome 🎉. Thanks a lot @trevor-bliss

  1. Updating the common tsconfig to commonjs fixed the issue 🥳
  2. I had fixed the incorrect main entry in package.json in 85609d3 but forgot to push 🤦. Sorry about that.
  3. I get what you are saying and guess am ok with it for now. Cleaned up code as per your suggestions. But sometime in the future would like to offer a simplified 1 step setup process rather than the 2 step process if thats something most consumers of the API would want and could benefit from. But if most of them already have a jest setup and jest config and have no issues with the 2 step process, we could leave it as is.
  4. Got the integrations tests with a different jest config to run from project root with the help of some config in the root jest config.

Thanks a lot for this!

mohanraj-r commented

Fixed by #9

AllanOricil commented

If you came here because you had this error, be aware that the Api has changed from registerA11yMatchers to setup while initializing these custom matchers

https://www.npmjs.com/package/@sa11y/jest

const { setup } = require('@sa11y/jest'); // CommonJS
import { setup } from '@sa11y/jest'; // ES6
// Register the sa11y matcher
setup();