feature(ruleset): add recommended, extended rulesets #3

mohanraj-r · 2020-03-13T02:55:52Z

Add recommended, extended preset rules
Refactor to use Project References · TypeScript
Refactor to switch to babel jest from ts-jest (Fixes Migrate from ts-jest to babel typescript transformer #4)
Add spell checker, typescript doc plugin

add single quote pref to editor config

from https://github.com/salesforce/oss-template

moved comments for enabled options from inline to their own line because prettier strips the whitespace in inline comments

this was resulting in errors from the typescript eslint plugin

trevor-bliss · 2020-03-23T18:38:52Z

packages/rules/__tests__/rules.test.ts

+
+describe('@sa11y/rules sanity checks with axe', () => {
+    // Rules that have been excluded from running due to being deprecated by axe
+    // or due to their experimental nature


What is the source of truth for this list? Or asked another way, how do we know when this list is stale and we need need to update it?

We have an internal doc that we maintain with the product accessibility team (@cordeliadillon & Jesse )
Currently making sure the rules here are in sync with the doc is a manual process.
I have been also wondering about how this could be automated to make sure it is updated.
Versioning the doc itself and maintaining a changelog in the doc along with current deployment status might help.
Or to ease maintenance and sync overhead could we make this repo the single source of truth and deprecate the doc @cordeliadillon ?

If there is a better process please let me know.

Since @cordeliadillon might be busy with the feature freeze support think it might be better to open an issue to deal with this a little later without blocking this PR ?

I don't consider this blocking for the PR, just want to make sure we have a plan going forward :). Issue is fine for now.

Thanks Trevor. Opened #5

trevor-bliss · 2020-03-23T18:42:51Z

packages/rules/lib/index.ts

-function rules() {
-    // TODO
-}
+export default extended;


How do users use just the "recommended" ruleset rather than the extended?

The recommended ruleset is also exported. Shouldn't that make it available if users want to import it. Sorry if I am missing something.

The assertAccessible and Jest APIs would default to extended and have an option to override to recommended. Guess it will become clear when those packages are added?

Yeah I think that makes sense to have it default to one with the option for the other.

trevor-bliss · 2020-03-23T18:49:26Z

Also, you didn't update the logic in this PR specifically but I don't think the way the dist folder is generated is correct. I'm not sure exactly how you'll end up delivering/publishing the packages yet but that should be revisited in a future PR.

mohanraj-r · 2020-03-23T19:31:41Z

Also, you didn't update the logic in this PR specifically but I don't think the way the dist folder is generated is correct. I'm not sure exactly how you'll end up delivering/publishing the packages yet but that should be revisited in a future PR.

@trevor-bliss Yes you are right, I wanted to check with you on this. If there are examples of monorepo Typescript projects that publish to npm in salesforce, please let me know - I will take a look.

trevor-bliss · 2020-03-23T20:37:30Z

@mohanraj-r It's relatively complex but the LWC monorepo is an example of multiple subpackages that use typescript and lerna.

https://github.com/salesforce/lwc/blob/master/package.json#L20
https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine/package.json#L14

I think the main thing for this project is the dist should be generated per project (packages/rules/dist) and the package.json in the rules project should update it's files entry to point to the local dist. It also doesn't make sense to publish the tests here in the dist.

to follow more widespread naming practices

add a tsconfig specific to rules, move dist to rules,

mohanraj-r · 2020-03-24T00:49:28Z

@mohanraj-r It's relatively complex but the LWC monorepo is an example of multiple subpackages that use typescript and lerna.
...
I think the main thing for this project is the dist should be generated per project (packages/rules/dist) and the package.json in the rules project should update it's files entry to point to the local dist. It also doesn't make sense to publish the tests here in the dist.

Thanks @trevor-bliss
Looked at a bunch of typescript lerna projects ..
Added a typescript project ref to the root tsconfig and added a tsconfig specific to rules package which puts the dist under the package dir.
Looks like there is currently no way to reuse the outDir and some related settings in tsconfig without duplicating them for each package as they are relative to the dir containing the tsconfig.
Also added exclusion for tests.

Btw, reg the package name

would preset-rules be better?
would putting published packages under packages/@sa11y/ be better to differentiate them from internal/unpublished packages (of which I don't see the need for now)

trevor-bliss · 2020-03-24T01:52:20Z

Btw, reg the package name

would preset-rules be better?

would putting published packages under packages/@sa11y/ be better to differentiate them from internal/unpublished packages (of which I don't see the need for now)

Yes, I think something like preset-rules or axe-config would be better than just "rules". When I see rules I assume there will be custom rules in here.

I wouldn't bother making a @sally folder until there are private unpublished packages in this repo. That's something you can change later if you need to.

to follow common naming conventions

to prevent double compilation - build time reduced in half

mohanraj-r · 2020-03-24T20:44:55Z

@trevor-bliss

Yes, I think something like preset-rules or axe-config would be better than just "rules". When I see rules I assume there will be custom rules in here.

Renamed package to preset-rules

I wouldn't bother making a @sally folder until there are private unpublished packages in this repo. That's something you can change later if you need to.

Makes sense - was thinking the same 👍

Also switched from ts-jest to babel jest.

mohanraj-r · 2020-03-24T23:23:14Z

@cordeliadillon I have merged the PR which adds the recommended, extended rules. When you get to it if you have any comments on this PR please let me know, I will take care of it in the next PR. Also when you get some time we can discuss about the source of truth for rulesets (#5) - not urgent but important.

Mohan Raj Rajamanickam added 29 commits March 12, 2020 14:49

chore: lerna init

05d6752

chore: typescript init

f623fb3

chore: set yarn as npm client for lerna

163a05b

chore: add dev deps typescript, jest

f7baef3

chore: add editor config

8cde185

chore: add git ignore

5baf5fb

chore: add typescript config with minimal overrides

c412301

chore: add dev dep eslint typescript plugin with minimal eslint config

85ac589

chore: add dev dep jest eslint plugin

343ca82

chore: add prettier eslint plugins, config

08f0a43

add single quote pref to editor config

chore: add scaffolding for ruleset package

1d7b7ea

chore: add yarn lock file

8a0ad5a

doc: Add readme with high-level outline of packages

df24f10

doc: Add license, code of conduct

5a080b8

from https://github.com/salesforce/oss-template

doc: add eslint plugin to check/fix copyright header

9ffae41

chore: update prettier config to sync with editorconfig

03ed0de

build: add deps, config to lint git commits

d3db063

build(root): add deps, config for typescript jest integration

98f723c

style(root): pretty format typescript config json

aca10f7

moved comments for enabled options from inline to their own line because prettier strips the whitespace in inline comments

feat(ruleset): add recommended, extended rulesets

3ab49ef

test(ruleset): temp disable failing assertion

d8a489a

ci(root): add ci workflow based on github actions

e194543

build(root): add build script alias, fix typescript config as req

06a4d35

build(root): add axe-core as dev dep

8e604f4

build(root): mark generated files for github

4407a99

build(root): remove tests exclusion from typescript config

c93a31d

this was resulting in errors from the typescript eslint plugin

refactor(rulesets): extract common parts out, reuse and remove duplicate

4d6182a

test(rulesets): add sanity checks for rules, add tests with axe

4f399e0

build(root): add explicit public npm registry to override internal

53bd6cf

mohanraj-r mentioned this pull request Mar 18, 2020

build(root): Initial project scaffolding #2

Merged

Mohan Raj Rajamanickam added 7 commits March 20, 2020 14:00

build(root): add back axe-core dependency that got removed when merging

d170edc

fix(rules): add missed out rule label-content-name-mismatch

2407938

style(root): lint docs - after merge

0b95a9e

build(root): add tsdoc eslint plugin dep, config

10c1aba

build(root): add code coverage opt, threshold, ci config

c4f6eba

refactor(rules): refactor rulesets based on interface type

aff0f70

docs(root): add changelog

dfbfcb6

mohanraj-r requested a review from a team March 20, 2020 22:52

mohanraj-r marked this pull request as ready for review March 20, 2020 22:52

fix(rules): minor doc, import fixes

e8ff557

trevor-bliss reviewed Mar 23, 2020

View reviewed changes

Mohan Raj Rajamanickam added 3 commits March 23, 2020 16:34

build(root): add typescript build info files to git ignore

182df1a

refactor(rules): mv source files under src from lib

c91e9e3

to follow more widespread naming practices

build(rules): add typescript project ref to rules

1506684

add a tsconfig specific to rules, move dist to rules,

build(root): add spell checker dep, config; add to git staged lint check

d2cd7e4

Mohan Raj Rajamanickam added 4 commits March 24, 2020 12:04

refactor(rules): rename rules package to preset-rules

8fe1eaa

to follow common naming conventions

refactor(root): split tsconfig module references into its own config

e125327

to prevent double compilation - build time reduced in half

build(root): add babel jest typescript deps, config

57e2d68

build(root): remove ts-jest deps as it is replaced by babel jest

512fbb6

trevor-bliss approved these changes Mar 24, 2020

View reviewed changes

mohanraj-r mentioned this pull request Mar 24, 2020

Source of truth for rulesets #5

Closed

mohanraj-r merged commit 3ff2af7 into master Mar 24, 2020

mohanraj-r deleted the feat/add_package_rulesets branch March 24, 2020 21:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(ruleset): add recommended, extended rulesets #3

feature(ruleset): add recommended, extended rulesets #3

mohanraj-r commented Mar 13, 2020 •

edited

Loading

trevor-bliss Mar 23, 2020

mohanraj-r Mar 23, 2020

mohanraj-r Mar 24, 2020

trevor-bliss Mar 24, 2020

mohanraj-r Mar 24, 2020

trevor-bliss Mar 23, 2020

mohanraj-r Mar 23, 2020

mohanraj-r Mar 23, 2020

trevor-bliss Mar 23, 2020

trevor-bliss commented Mar 23, 2020

mohanraj-r commented Mar 23, 2020

trevor-bliss commented Mar 23, 2020 •

edited

Loading

mohanraj-r commented Mar 24, 2020

trevor-bliss commented Mar 24, 2020

mohanraj-r commented Mar 24, 2020

mohanraj-r commented Mar 24, 2020

feature(ruleset): add recommended, extended rulesets #3

feature(ruleset): add recommended, extended rulesets #3

Conversation

mohanraj-r commented Mar 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trevor-bliss commented Mar 23, 2020

mohanraj-r commented Mar 23, 2020

trevor-bliss commented Mar 23, 2020 • edited Loading

mohanraj-r commented Mar 24, 2020

trevor-bliss commented Mar 24, 2020

mohanraj-r commented Mar 24, 2020

mohanraj-r commented Mar 24, 2020

mohanraj-r commented Mar 13, 2020 •

edited

Loading

trevor-bliss commented Mar 23, 2020 •

edited

Loading