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

test: [M3-7820] - Suppress Cypress "MODULE_LEVEL_DIRECTIVE" and "SOURCEMAP_ERROR" warnings #10239

Merged

Conversation

jdamore-linode
Copy link
Contributor

Description πŸ“

Recent improvements to Cloud Manager's dev tools introduced various MODULE_LEVEL_DIRECTIVE and SOURCEMAP_ERROR console warnings when Cypress builds each spec file via Vite.

The cause of these warnings is relatively well-understood[1] and the underlying issue(s) are harmless and irrelevant from a Cloud Manager development perspective. This PR addresses the issue by using the @vitejs/react-plugin-swc plugin to suppress the MODULE_LEVEL_DIRECTIVE warning, and by manually supressing the related SOURCEMAP_ERROR warnings.

(1) MUI contains 'use client' module-level directives, which are intended to allow library developers to denote client-side and server-side code. Meanwhile, Rollup (and therefore Vite) does not support module-level directives, hence the warning.

Changes πŸ”„

  • Use @vitejs/plugin-react-swc in Cypress Vite config to suppress MODULE_LEVEL_DIRECTIVE warnings
  • Manually suppress SOURCEMAP_ERROR Rollup warnings in Cypress Vite config

Preview πŸ“·

Before After
Screenshot 2024-02-28 at 1 27 58 PM Screenshot 2024-02-28 at 1 25 48 PM

|

How to test πŸ§ͺ

We can rely on the automated tests for this. We just want to confirm that the tests continue to pass and that all of the relevant console warnings have been suppressed.

As an Author I have considered πŸ€”

Check all that apply

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@jdamore-linode jdamore-linode self-assigned this Feb 28, 2024
@jdamore-linode jdamore-linode requested a review from a team as a code owner February 28, 2024 18:29
@jdamore-linode jdamore-linode requested review from cliu-akamai and removed request for a team February 28, 2024 18:29
@jdamore-linode jdamore-linode requested a review from a team as a code owner February 28, 2024 18:32
@jdamore-linode jdamore-linode requested review from bnussman-akamai and hkhalil-akamai and removed request for a team February 28, 2024 18:32
Copy link

github-actions bot commented Feb 28, 2024

Coverage Report: βœ…
Base Coverage: 81.34%
Current Coverage: 81.34%

@@ -1,10 +1,36 @@
import react from '@vitejs/plugin-react-swc';
import svgr from 'vite-plugin-svgr';
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this package related?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I imported both of these plugins to mirror what we're doing in Cloud's Vite config.

plugin-react-swc is really the crucial one, but I opted to include svgr as well for two reasons:

  • As a safeguard in case we ever import a file in src from within a Cypress test, which in turn imports an SVG file (or imports a module which imports a module which imports an SVG, etc.). I would expect that to either cause a warning to get logged, or possibly even prevent the spec from building
  • I figure since it isn't harmful and it makes us more consistent with Cloud's config, why not πŸ€·β€β™‚οΈπŸ˜…

@@ -1,10 +1,36 @@
import react from '@vitejs/plugin-react-swc';
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any tradeoffs to using swc -- i.e., build differences between e2e and prod?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but only because Cloud also uses @vitejs/plugin-react-swc!

I think we opted to use that for performance reasons (swc is fast), and so I used it here too, but both @vitejs/plugin-react and @vitejs/plugin-react-swc contain the exact same log suppression logic (so either would really work here for the purpose of this ticket).

One thing to note is that this Vite config is only used when building the Cypress *.spec.ts files -- Cloud itself is always built using the Vite config in packages/manager, so while consistency between the configs is good, we're not at risk of ending up in a situation where e.g. Cloud is built differently when running our Cypress tests, resulting in bugs being present (or absent) in our tests but not in Production or vice-versa.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh didn't know that. Thanks for the explanation.

Comment on lines +27 to +29
if (warning.code === 'SOURCEMAP_ERROR') {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason not to suppress MODULE_LEVEL_DIRECTIVE the same way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the MODULE_LEVEL_DIRECTIVE warning is suppressed by the @vitejs/plugin-react-swc plugin. We could manually suppress it and drop the React and SVGR plugins, but I opted to do it this way since we already have these dependencies installed and we're importing more and more src code from the Cypress tests.

I appreciate these good questions, @hkhalil-akamai!

Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Thanks for answering all my questions. Just wanted to make sure I had a complete understanding. Great clean up!

@jdamore-linode jdamore-linode added Add'tl Approval Needed Waiting on another approval! and removed Ready for Review labels Feb 28, 2024
Copy link
Contributor

@abailly-akamai abailly-akamai left a comment

Choose a reason for hiding this comment

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

Thanks for suppressing those! 🧹

Glad I came here, great questions and answers πŸ‘

@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Feb 29, 2024
@jdamore-linode jdamore-linode merged commit 525aed0 into linode:develop Feb 29, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants