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.

Sign up for GitHub

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

Jump to bottom

fix: colorio.js patch mocking CSS #4456

Merged
merged 42 commits into from
Jun 13, 2024
Merged

fix: colorio.js patch mocking CSS #4456

Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
6a1f4ec
chore: bump the npm-low-risk group across 1 directory with 9 updates
dependabot[bot] May 6, 2024
d391a0c
install colorjs.io 0.4.3 exactly
straker May 14, 2024
2ef636d
Merge branch 'develop' into dependabot/npm_and_yarn/npm-low-risk-f6f5…
straker May 14, 2024
c62613f
fix: colorio.js patch mocking CSS
gaiety-deque May 9, 2024
920c990
import local colorjs instead of CDN
gaiety-deque May 10, 2024
576dc99
copy in an unpatched file for testing against
gaiety-deque May 10, 2024
5c39c29
patch colorjs with optional chaining to support CSS null
gaiety-deque May 10, 2024
841fb7d
removed commented out html script
gaiety-deque May 10, 2024
69b80f5
:robot: Automated formatting fixes
gaiety-deque May 10, 2024
edf803a
debugging issue in CI
gaiety-deque May 10, 2024
3a7b7e6
still debugging in CI
gaiety-deque May 10, 2024
eff6ad3
undoes changes to prototype
gaiety-deque May 10, 2024
4fada62
patch is a build step
gaiety-deque May 13, 2024
71cc2b4
smaller patch, for 0.4.5
gaiety-deque May 13, 2024
ebd2e96
move to grunt build step instead of prebuild postbuild
gaiety-deque May 13, 2024
57de085
:robot: Automated formatting fixes
gaiety-deque May 13, 2024
89b7386
patch only color.js
gaiety-deque May 14, 2024
4e6682d
correct import paths for proxied color.js in karma
gaiety-deque May 14, 2024
0cb35e9
testing a fix in CI, revert
gaiety-deque May 14, 2024
af79b8b
Revert "testing a fix in CI, revert"
gaiety-deque May 14, 2024
2a4d76c
add unpatched to build cache
gaiety-deque May 14, 2024
20a8a6f
add colorjs patched dist to build cache as well
gaiety-deque May 14, 2024
c5444fd
:robot: Automated formatting fixes
gaiety-deque May 14, 2024
ac45ddf
patch 0.4.3 instead of 0.4.5
gaiety-deque May 14, 2024
208377a
patch test now a core test instead of integration
gaiety-deque May 16, 2024
acbbc38
Merge branch 'develop' into colorjs-patch
gaiety-deque May 16, 2024
ad0d80e
Merge branch 'develop' into colorjs-patch
gaiety-deque May 17, 2024
64be6d1
package lock correction
gaiety-deque May 17, 2024
b79bae4
Merge branch 'develop' into colorjs-patch
gaiety-deque May 20, 2024
1785f2c
move patch steps to package pre/post steps not in grunt
gaiety-deque May 20, 2024
e1c1eae
Merge branch 'colorjs-patch' of github.com:dequelabs/axe-core into co…
gaiety-deque May 20, 2024
87c7845
removed file deleted from eslint merge issue
gaiety-deque May 20, 2024
8b78165
test axe*.js, npm prepare
gaiety-deque May 28, 2024
ae68dc8
cii now runs prepare step for tests
gaiety-deque May 28, 2024
a2842f4
circle config runs prepare
gaiety-deque May 28, 2024
3ae8d35
npm run prepare, not npm prepare
gaiety-deque May 28, 2024
9c255b1
Merge branch 'develop' into colorjs-patch
gaiety-deque May 28, 2024
3271c2e
Merge branch 'develop' into colorjs-patch
gaiety-deque Jun 4, 2024
434173b
proper test validating the patch
gaiety-deque Jun 6, 2024
283dca9
:robot: Automated formatting fixes
gaiety-deque Jun 6, 2024
09a36d5
no need to unpatch
gaiety-deque Jun 7, 2024
ca3091a
Merge branch 'colorjs-patch' of github.com:dequelabs/axe-core into co…
gaiety-deque Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ jobs:
paths:
- node_modules

# Build and cache axe.js
# Build and cache built files
build_unix:
<<: *defaults
<<: *unix_box
Expand All @@ -74,6 +74,8 @@ jobs:
paths:
- axe.js
- axe.min.js
- patches/unpatched
- node_modules/colorjs.io/dist/color.js

# Run ESLINT
lint:
Expand Down
12 changes: 12 additions & 0 deletions .eslintignore
View file
Open in desktop
gaiety-deque marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
**/node_modules/*
**/tmp/*
patches/*

build/tasks/aria-supported.js

doc/api/*
doc/examples/jest_react/*.js

lib/core/imports/*.js
axe.js
axe.min.js
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,5 @@ typings/axe-core/axe-core-tests.js
# doc
doc/rule-descriptions.*.md

# unpatched files, made from a build step
patches/unpatched/*
22 changes: 21 additions & 1 deletion Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ camelcase: ["error", {"properties": "never"}]
module.exports = function (grunt) {
'use strict';

grunt.loadNpmTasks('grunt-exec');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, we're going to have to stop adding tasks to Grunt if we're ever going to get rid of it.

Should now be that time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be happy too, if we know what we'd rather do instead. @WilcoFiers suggested this should be in the build step. Thus there are multiple ways we could do this:

  • As-is in the grunt build step
  • Another custom node script like we do for several other scripts referenced in the package.json
  • Tacked onto the build command in package.json with && cp path... && npx patch-package name... etc
  • A different build tool, but that blows up the scope of this task

I'm of the opinion that build tools are changing all the time and it makes it feel strange to suggest just switching to the new hotness, but that just writing more custom node scripts in /build/* results in us reinventing the wheel. Still, moving towards vite/esbuild etc in the future may be my minor preference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we want this to be part of the build it needs to go in the grunt build step so npm develop builds it as well. We won't be moving off grunt any time soon and not in this PR so a grunt step is fine for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I said it needed to be in grunt :P. We did a good while back agree not to add more grunt tasks if we could at all avoid it. It seems viable to put this work in prebuild / predevelop scripts instead. Did you try that @gaiety-deque? If that was more difficult I can live with a Grunt solution, but if we can avoid these extra dependencies + more grunt tasks I think that's the way to go on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies @WilcoFiers I didn't mean to imply that you explicitly said it had to be in the grunt build step, just that the issue requested it be a part of the build step. Since our build step just calls "grunt" currently and the pre/post build steps were more test-like in nature it felt like we were intentionally doing these things in grunt.

Made a change 1785f2c to move it to be commands run in the package json scripts.

grunt.loadNpmTasks('grunt-babel');
grunt.loadNpmTasks('grunt-bytesize');
grunt.loadNpmTasks('grunt-contrib-clean');
grunt.loadNpmTasks('grunt-contrib-concat');
grunt.loadNpmTasks('grunt-contrib-copy');
grunt.loadNpmTasks('grunt-contrib-uglify');
grunt.loadNpmTasks('grunt-contrib-watch');
grunt.loadNpmTasks('grunt-bytesize');
grunt.loadTasks('build/tasks');

var langs;
Expand Down Expand Up @@ -184,6 +186,22 @@ module.exports = function (grunt) {
src: 'lib/rules/**/*.json'
}
},
copy: {
main: {
expand: true,
cwd: 'node_modules/colorjs.io/dist/',
src: 'color.js',
dest: 'patches/unpatched/'
}
},
exec: {
unpatch: {
command: 'npx patch-package --reverse'
},
patch: {
command: 'npx patch-package'
}
},
uglify: {
beautify: {
files: langs.map(function (lang, i) {
Expand Down Expand Up @@ -268,9 +286,11 @@ module.exports = function (grunt) {
'esbuild',
'add-locale:newLang'
]);
grunt.registerTask('patch', ['exec:unpatch', 'copy', 'exec:patch']);
grunt.registerTask('build', [
'clean:core',
'validate',
'patch',
'metadata-function-map',
'esbuild',
'configure',
Expand Down
1 change: 1 addition & 0 deletions eslint.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ module.exports = [
ignores: [
'**/node_modules/*',
'**/tmp/*',
'patches/*',
'build/tasks/aria-supported.js',
'doc/api/*',
'doc/examples/jest_react/*.js',
Expand Down