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

Allow multiple --css flags in cli fixes #514 #515

Closed
wants to merge 5 commits into from

Conversation

josenobile
Copy link

Multiple --css flags were lost in v2.0, this restores the possibility of multiple --css flags
Fixes #514

@josenobile
Copy link
Author

josenobile commented Aug 20, 2021

Hello @bezoerb , can you please give a quick look if the acceptance tests are failing due to the isMultiple attribute added to --css flag?

@josenobile
Copy link
Author

node --version
v14.18.2
npm --version
8.3.0
Linux 4.18.0-305.19.1.lve.el7h.x86_64 (CloudLinux 7.9)

npm test

[email protected] test
npm run xo && npm run jest --runInBand

[email protected] xo
xo

Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating

src/file.js:561:1
⚠ 561:1 Function getStylesheetPath has a complexity of 21. Maximum allowed is 20. complexity
⚠ 771:1 Async function getStylesheet has a complexity of 31. Maximum allowed is 20. complexity

2 warnings

[email protected] jest
jest --coverage

Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating
PASS test/config.test.js
PASS test/array.test.js
PASS test/file.test.js
PASS test/core.test.js
PASS test/index.test.js (9.862 s)
PASS test/cli.test.js (17.468 s)
PASS test/blackbox.test.js (30.51 s)
----------------------|---------|----------|---------|---------|----------------------------------------------------------------------------------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------------------|---------|----------|---------|---------|----------------------------------------------------------------------------------------------------
All files | 93.55 | 85.03 | 93.7 | 93.83 |
critical | 87.69 | 81.82 | 88.46 | 87.5 |
cli.js | 82.22 | 76.27 | 85 | 81.25 | 184-187,215,221,228-239,247
index.js | 100 | 100 | 100 | 100 |
critical/src | 94.59 | 85.4 | 94.68 | 94.92 |
array.js | 95 | 80 | 75 | 95 | 31
config.js | 94.44 | 82.22 | 100 | 94.44 | 108,110
core.js | 98.92 | 87.1 | 100 | 98.89 | 71
errors.js | 100 | 70 | 100 | 100 | 16-47
file.js | 93.43 | 86.06 | 95.65 | 93.89 | 86,168,185,197,217,245-246,268,272,334,353-354,367,378,456,510-512,528,599,621,626,651,783,840-841
critical/test/helper | 100 | 100 | 100 | 100 |
index.js | 100 | 100 | 100 | 100 |
----------------------|---------|----------|---------|---------|----------------------------------------------------------------------------------------------------

Test Suites: 7 passed, 7 total
Tests: 2 skipped, 151 passed, 153 total
Snapshots: 0 total
Time: 30.776 s
Ran all test suites.

@bezoerb
Copy link
Collaborator

bezoerb commented Jan 4, 2022

Hey @josenobile, sorry for the late reply.
It seems like the isMultiple is causing the error.
When the css option is set, critical doesn't process the css files listed in the html source. Due to the isMultiple flag the css option is an empty array instead of undefined See

if (css) {

I think you should either unset the css option in cli.js when it's an empty array or the check in file.js needs to be modified

@josenobile
Copy link
Author

Hello @bezoerb, Can you please help me to check why it fails two tests? 'Take html file piped to critical' and 'Pipe html file inside a folder to critical'

@bezoerb
Copy link
Collaborator

bezoerb commented Oct 3, 2022

Hey @josenobile,
sorry for the late response. I found the error. See linked PR

@bezoerb bezoerb closed this Oct 3, 2022
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.

Bug: The flag --css can only be set once
2 participants