-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix: allow next.js apps to import global styles #22769
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment!
@@ -1 +1,4 @@ | |||
// | |||
// Importing global styles fails with Next.js due to restrictions on style imports. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to add some kind of assertion that the global style is actually applied to some element?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an assertion here: b0e8bf0
@ZachJW34 - I tried to install the binary (to test your changes) built by Circle CI from this PR via the provided command: npm install https://cdn.cypress.io/beta/npm/10.3.1/linux-x64/zachw/issue-22525-a049ed7f3a2f9a709e466f60bf2da775312af521/cypress.tgz But the install failed to install on my local nextjs-playground project with a NPM install Cypress error output$ npm install https://cdn.cypress.io/beta/npm/10.3.1/linux-x64/zachw/issue-22525-a049ed7f3a2f9a709e466f60bf2da775312af521/cypress.tgz
npm ERR! code 1
npm ERR! path /Users/sayvai/repos/Sayvai/nextjs-playground/node_modules/cypress
npm ERR! command failed
npm ERR! command sh -c node index.js --exec install
npm ERR! ⚠ Warning: You are installing a pre-release build of Cypress.
npm ERR!
npm ERR! Bugs may be present which do not exist in production builds.
npm ERR!
npm ERR! This build was created from:
npm ERR! * Commit SHA: a049ed7f3a2f9a709e466f60bf2da775312af521
npm ERR! * Commit Branch: zachw/issue-22525
npm ERR! * Commit Timestamp: 2022-07-13T19:10:51.000Z
npm ERR!
npm ERR! Installing Cypress (version: https://cdn.cypress.io/beta/binary/10.3.1/darwin-x64/zachw/issue-22525-a049ed7f3a2f9a709e466f60bf2da775312af521/cypress.zip)
npm ERR!
npm ERR! [STARTED] Task without title.
npm ERR! The Cypress App could not be downloaded.
npm ERR!
npm ERR! Does your workplace require a proxy to be used to access the Internet? If so, you must configure the HTTP_PROXY environment variable before downloading Cypress. Read more: https://on.cypress.io/proxy-configuration
npm ERR!
npm ERR! Otherwise, please check network connectivity and try again:
npm ERR!
npm ERR! ----------
npm ERR!
npm ERR! URL: https://cdn.cypress.io/beta/binary/10.3.1/darwin-x64/zachw/issue-22525-a049ed7f3a2f9a709e466f60bf2da775312af521/cypress.zip
npm ERR! Error: Failed downloading the Cypress binary.
npm ERR! Response code: 404
npm ERR! Response message: Not Found
npm ERR!
npm ERR! ----------
npm ERR!
npm ERR! Platform: darwin-x64 (21.5.0)
npm ERR! Cypress Version: 10.3.1
npm ERR! [FAILED] The Cypress App could not be downloaded.
npm ERR! [FAILED]
npm ERR! [FAILED] Does your workplace require a proxy to be used to access the Internet? If so, you must configure the HTTP_PROXY environment variable before downloading Cypress. Read more: https://on.cypress.io/proxy-configuration
npm ERR! [FAILED]
npm ERR! [FAILED] Otherwise, please check network connectivity and try again:
npm ERR! [FAILED]
npm ERR! [FAILED] ----------
npm ERR! [FAILED]
npm ERR! [FAILED] URL: https://cdn.cypress.io/beta/binary/10.3.1/darwin-x64/zachw/issue-22525-a049ed7f3a2f9a709e466f60bf2da775312af521/cypress.zip
npm ERR! [FAILED] Error: Failed downloading the Cypress binary.
npm ERR! [FAILED] Response code: 404
npm ERR! [FAILED] Response message: Not Found
npm ERR! [FAILED]
npm ERR! [FAILED] ----------
npm ERR! [FAILED]
npm ERR! [FAILED] Platform: darwin-x64 (21.5.0)
npm ERR! [FAILED] Cypress Version: 10.3.1
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/sayvai/.npm/_logs/2022-07-14T07_58_33_424Z-debug-0.log Also, when i looked further into the CI Circle workflow build pipeline for the Build the Cypress binary job, although it may have appeared successful, the underlying console output appears to error out on the final build step. I noticed some workflow checks on this PR failed, and so not sure whether those failed checks relates to the build failure? 🤔 |
@Sayvai Not sure why the binary isn't loading. I've pushed another commit and will see if I can figure out what's wrong when the new binary is published. The logs at the end of the binary build job are misleading. CI runs the binary against a test that is expected to fail so it prints some expected error messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not test on my machine yet but my comments are addressed, so going to approve. I can give it a try as well.
@ZachJW34 - Yeah, i am still having trouble installing the build generated from this PR with the same console error output (described in earlier comment). My theory is that it could be a private registry permissions auth issue, or i need some special localised npm configuration to access builds from third-party (non-default) registry, albeit via the Cypress CDN 🤔 |
@Sayvai Added the build for |
@ZachJW34 - I've finally managed to download the build via the cypress-bot provided npm install command / URL. Thanks for fixing the build! Now, when i re-ran the test with the PR build, the Cypress browser window still renders out the message And if i comment out the global CSS import from within Am i missing another step (configuration) with this change? 🤔 |
I checked out your repo and the Did you happen to install the new binary when you had an active Cypress instance? If not, can you give post a little more info on how I can reproduce what you are seeing? |
@ZachJW34 - I installed the new binary after uninstalling the stable binary. Here's the reproduction steps i undertook:
Cypress Terminal Output$ npm run cypress:open
> [email protected] cypress:open
> ./node_modules/.bin/cypress open
Missing baseUrl in compilerOptions. tsconfig-paths will be skipped
[@cypress/webpack-dev-server]: removing HotModuleReplacementPlugin from configuration.
<i> [webpack-dev-server] Project is running at:
<i> [webpack-dev-server] Loopback: http://127.0.0.1:8080/
<i> [webpack-dev-server] Content not from webpack is served from '/Users/sayvai/repos/Sayvai/nextjs-playground/public' directory
13 assets
137 modules
ERROR in ./components/layout/layout.module.scss
No template for dependency: ModuleHotAcceptDependency
ERROR in ./styles/globals.scss
No template for dependency: ModuleHotAcceptDependency
client (webpack 5.73.0) compiled with 2 errors in 868 ms
Have i gone wrong in any of the above steps? Are you able to reproduce the error i observed? 🤔 |
I had the exact same result as @ZachJW34 - down to the "comment in, comment out" part, see my post here. This weird behavior was without this branch. I am not sure why that is happening. I did not try on this branch, yet. |
@Sayvai thanks for the thorough reproduction. Unfortunately, I can't reproduce this issue following your steps (though I don't have any SSL certs to copy). @lmiller1990 when you pull this down, can you go through the reproduction steps provided by @Sayvai and see if you are experiencing any issues? |
@Sayvai Okay I was able to reproduce this. The issue seems to be related to the |
@Sayvai I'm hoping it is fixed now. Can you try one of the newer binaries ( You might need to blow away the next cache via |
@ZachJW34 - Bingo! You were right to point out the additional caching issue too with Next.js. I can now confidently confirm that your latest fixes fixed the issue completely, as i can see the global styles now applied within the Cypress Chrome test runner window (screenshot below), and i also no longer see any further error output in the terminal sessions either. I would like to thank @ZachJW34 and @lmiller1990 for assisting me with this I'll look forward to the next patch release cut (assumed Many thanks both 🎉 |
@Sayvai thanks for finding the issue and the words of encouragement! It goes a long way ❤️ Like you said, this should be available in the next release of Cypress! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one potential comment/guard worth adding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good nice work!
…guide Issue was fix in cypress-io/cypress#22769
…guide (#4771) Issue was fix in cypress-io/cypress#22769
User facing changelog
Allow Next.js apps to import global styles
Additional details
Next.js restricts the imports of global css/scss files to the top-level
<App />
component. The Webpack configuration for these rules can be found here. This PR removes the limitations that Next applies to global styles, allowing their import in the support file.Steps to test
I tweaked the
next
project fixture to include global style imports. If you remove the call to allowGlobalStylesImports and rebuild the@cypress/webpack-dev-server package
, thenpm/webpack-dev-server/cypress/e2e/next.cy.ts
system tests will fail.How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?