Skip to content
This repository has been archived by the owner on Sep 16, 2023. It is now read-only.

Reference Error when trying to use it with penthouse, babel 7 and yarn workspaces #675

Closed
batusai513 opened this issue Nov 23, 2018 · 9 comments
Labels

Comments

@batusai513
Copy link

batusai513 commented Nov 23, 2018

First, thanks for this library, and the effort building it.

I'm getting a Reference error after I try to extract the critical css with penthouse (It uses puppeteer underneath) with babel 7 (@babel/register) inside a Yarn workspace.

Right now we have an intricate deploying setup that generates html on the server side with react and then creates a server to extract the critical css, everything works well until the function pruneNonCriticalSelectors is called in here, it gets evaluated by puppeteer and throws the following reference error:

Error: Evaluation failed: ReferenceError: _c91‍ is not defined
    at pruneNonCriticalSelectors (__puppeteer_evaluation_script__:5:3)
    at ExecutionContext.evaluateHandle (/Users/richard.roncancio/projects/dynamic-step/node_modules/puppeteer/lib/ExecutionContext.js:106:13)
    at process.internalTickCallback (internal/process/next_tick.js:77:7)
(node:85760) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'goto' of undefined
    at loadPage (/Users/richard.roncancio/projects/dynamic-step/node_modules/penthouse/lib/core.js:428:30)
    at /Users/richard.roncancio/projects/dynamic-step/node_modules/penthouse/lib/core.js:260:33
    at Generator.next (<anonymous>)
    at step (/Users/richard.roncancio/projects/dynamic-step/node_modules/penthouse/lib/core.js:407:191)
    at /Users/richard.roncancio/projects/dynamic-step/node_modules/penthouse/lib/core.js:407:361

after debugging I've found that the console statements are prefixed with some gibberish that I assume that is babel related, I tried, among others, disable the cache of both babel register and esm, and it didn't work:

function pruneNonCriticalSelectors({
  selectors,
  maxElementsToCheckPerSelector
}) {
  _c91‍.g.console.log('debug: pruneNonCriticalSelectors init');
  var h = window.innerHeight;

  // cache whether elements are above fold,
  // primarily because getBoundingClientRect() can be slow to query,
  // and some stylesheets have lots of generic selectors (like '.button', '.fa' etc)
  var isElementAboveFoldCache = new Map();

  function isElementAboveFold(element) {
    if (isElementAboveFoldCache.has(element)) {
      return isElementAboveFoldCache.get(element);
    }

    // temporarily force clear none in order to catch elements that clear previous
    // content themselves and who w/o their styles could show up unstyled in above
    // the fold content (if they rely on f.e. 'clear:both;' to clear some main content)
    var originalClearStyle = element.style.clear || '';
    element.style.clear = 'none';
    var aboveFold = element.getBoundingClientRect().top < h;
    // cache so we dont have to re-query DOM for this value
    isElementAboveFoldCache.set(element, aboveFold);

    // set clear style back to what it was
    element.style.clear = originalClearStyle;

    // Should not be needed anymore with Chrome Headless:
    // do some monitoring before complete removing the code (below)
    // if (!aboveFold) {
    //   // phantomJS/QT browser has some bugs regarding fixed position;
    //   // sometimes positioning elements outside of screen incorrectly.
    //   // just keep all fixed position elements - normally very few in a stylesheet anyway
    //   var styles = window.getComputedStyle(element, null)
    //   if (styles.position === 'fixed') {
    //     console.log('debug: force keeping fixed position styles')
    //     return true
    //   }
    // }
    return aboveFold;
  }

  function isSelectorCritical(selector) {
    // we have a selector to test, first grab any matching elements
    let elements;
    try {
      elements = document.querySelectorAll(selector);
    } catch (e) {
      // not a valid selector, remove it.
      return false;
    }

    let nrElementsToCheck = elements.length;
    if (maxElementsToCheckPerSelector && nrElementsToCheck > maxElementsToCheckPerSelector) {
      _c91‍.g.console.log(`debug: isSelectorCritical, selector: ${selector} appearing ${nrElementsToCheck} time on page, ONLY checking first ${maxElementsToCheckPerSelector}...`);
      nrElementsToCheck = maxElementsToCheckPerSelector;
    }

    // only keep selectors that match at least one elements on the page above the fold
    for (let idx = 0; idx < nrElementsToCheck; idx++) {
      if (isElementAboveFold(elements[idx])) {
        return true;
      }
    }

    return false;
  }

  function filterSelectors(selectors) {
    _c91‍.g.console.log('debug: filterSelectors START');

    selectors = selectors.filter(isSelectorCritical);

    _c91‍.g.console.log('debug: filterSelectors DONE');
    return selectors;
  }

  return filterSelectors(selectors);
}

I've tried on another branch on our project that doesn't have set up the workspace and the latest babel and it completes the flow as expected, that's why i thing this is related with babel or the workspace.

Any hint would be really apreciated.

Thanks.

@jdalton
Copy link
Member

jdalton commented Nov 23, 2018

Hi @batusai513!

If you're running into things like _c91., that's where esm puts its runtime library methods, this means you're running transformed esm code. How are you getting access to that code and passing it to puppeteer to evaluate?

@batusai513
Copy link
Author

batusai513 commented Nov 23, 2018

Hi @jdalton

We are starting everything with the following script export NODE_ENV=production && node -r esm -r ./babel-register.js internals/scripts/deploy

inside that deploy file we are bundling the client side app with webpack, creating an express server to and creating the html on the server side, and finally extracting the critical css with penthouse, that's the basic flow we are following.

I don't access that code directly, That one es passed as a callback to puppeteer via penthouse the lib.

@jdalton
Copy link
Member

jdalton commented Nov 24, 2018

Ah good to know. It is likely toString()'ing the callback to evaluate it and getting the transform that way. I'll noodle on it for a second.

@batusai513
Copy link
Author

Yeah, but the little catch is that it works with Babel 6 / not a workspace environment, this are the two things that diverge from the non working branch.

@jdalton
Copy link
Member

jdalton commented Nov 24, 2018

Naw, that's just a bit of side noise. The issue is 💯 how Puppeteer toStrings code to evaluate. The workspace thing is causing code to be run untransformed vs. transformed so not hitting the issue. You can verify this by capturing the func.toString() source of the function passed to Puppeteer and writing it to a file to inspect. You'll run into this with any function passed to Puppeteer that references vars outside the scope of the function that doesn't exist in the page puppeteer is evaluating in.

@batusai513
Copy link
Author

@jdalton

I created a small demo repo showing the differences between babel 6 and babel 7, https://github.com/batusai513/penthouse-ex, when running the example with the babel 6 branch everything works well, when running it with babel 7 it shows the aforementioned error, hopefully this will help a bit.

Thanks.

@jdalton
Copy link
Member

jdalton commented Nov 24, 2018

No problem! Your repro validated my hunch.

@batusai513
Copy link
Author

@jdalton Is there something I can do on my side to try to mitigate this? What would you recommend me to do?

Thanks.

@jdalton
Copy link
Member

jdalton commented Nov 28, 2018

Nothing for you to do. I'm just noodling on how to handle this.

Update:

Patched 39b2007

@jdalton jdalton closed this as completed Nov 28, 2018
@jdalton jdalton added bug and removed question labels Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

2 participants