Not CSS needed anymore

[timonbandit]

Hey @hummal, your software is awesome but i think it miss some things. I've added the options to work without inline css. It's almost the same performance as before. And some (all) dependencies are too old. And benchmark.js was broken.

[philipp-winterle]

Thanks for your work. Please review the requested changes and update the pr. Also we need to adjust the github actions to remove node 10 support if you update to fs.extra 10. Update readme according to that

[timonbandit]

Hey @hummal - Done!

[timonbandit]

@hummal Please check tests. Works. At least for me :-)

[timonbandit]

@hummal Can you help me a little. I'm not sure I understood some tests. For example - MediaQuery 1024 selectors should be included And it's work, but you expect to get 0 elements. The same for MediaQuery 800 selectors should be included

[philipp-winterle]

About the tests. It counts the missing selectors by comparing the result set of selectors with the array of needed selectors. If one selector of expected selectors is missing it will be added to missingSelectors.

missingSelectors has to be 0 to be good

test("MediaQuery 1024 selectors should be included", () => {
    const missingSelectors = [];
    const selectorPrefix = "all and (min-width: 1024px)===";
    for (const selector of mustHaveSelectors.media1024) {
        if (!criticalSelectorRules.has(selectorPrefix + selector)) {
            missingSelectors.push(selectorPrefix + selector);
        }
    }
    expect(missingSelectors).toHaveLength(0);
});

[timonbandit]

Will try to update a bit latter!

[timonbandit]

Unfortunately i had to remove "vendor prefixestest case. It doesn't work withnocss` solution. and i don't want to extract it. To much problems for such small case.

[philipp-winterle]

Sorry for delay. Will have a look the next time

[philipp-winterle]

Ah I see you already released a new package on npm.

[timonbandit]

Hey @hummal Yes. I did the release. Because the current version is impossible to use from npm with puppeteer and no-css. I think I'll close this PR and prepare a new one. Without package.json changes and etc.

[timonbandit]

Please feel free to close it. Sorry for inconvenience

[philipp-winterle]

Hey @hummal Yes. I did the release. Because the current version is impossible to use from npm with puppeteer and no-css.

You could have used your repository link in package.json for that.

Iam currently working at your fork to help with the tests. If you reset the package.json and have a look at the lodash merge thingy Iam sure we can release it this weekend

[timonbandit]

Unfortunately I have no time this weekend :-( Just FYI - i tried to make critical generation microservice based on Google Cloud Functions. but seems it's not a good place to run crittr. any page generation took more 30second. Localy it works like a charm with external puppeteer. 1200 pages generation took around 15 min. But on Google cloud functions 15 min for 100 pages :-)

[philipp-winterle]

Okay, if you could fix the package.json stuff I will look on the rest.

Microservice: I was thinking of dealing with AWS and crittr. So similar approach. Problem is puppeteer. Too heavy for lambda and co. Not sure there is a way arround.

[timonbandit]

Okay, if you could fix the package.json stuff I will look on the rest.

Microservice: I was thinking of dealing with AWS and crittr. So similar approach. Problem is puppeteer. Too heavy for lambda and co. Not sure there is a way arround.

I did it. Nodejs+express+bullMQ+crittr+Redis. Works perfectly. But on local PC or on docker.

[philipp-winterle]

It is the cloud platform that is not a good match for puppeteer. You can read it in Google Lighthouse blogs where they are talking about that topic.

[philipp-winterle]

I changed your stuff. have a look if you agree. I resetted the package.json aswell.

And please test the puppeteer deepmerge problem. I included the isPlainObject function. This may already solve the problem.

[timonbandit]

I changed your stuff. have a look if you agree. I resetted the package.json aswell.

And please test the puppeteer deepmerge problem. I included the isPlainObject function. This may already solve the problem.

Thank you! I'll have a look on Monday!

[philipp-winterle]

@timonbandit did you had time? Wanna proceed ?

[timonbandit]

@timonbandit did you had time? Wanna proceed ?

@hummal Hey! Sorry for the delay! Extremely busy week - On my job we're preparing for Black Friday - working without Holidays :-) I think next week. Feel free to close the PR. I'll open the new one

[philipp-winterle]

@timonbandit all cool. relax. we dont need to open a new one. Everything is fixed here. You only need to check if you can use the browser option with deepmerge.

[timonbandit]

@hummal - Updated. Please check if you have time :-)