-
Notifications
You must be signed in to change notification settings - Fork 201
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
Do not compile NPMPackages which are used during testing #543
Conversation
} | ||
|
||
// scripts to be injected into the running test | ||
export const ENVIRONMENT_SCRIPTS: NPMPackage[] = [ |
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.
What if we checked for a URL parameter like ?nocompile, and have WCT add that to the URLs it injects? Maybe that separates concerns better?
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.
Oooh I like that solution. Will take a stab at that 👍
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.
+1 to not adding more WCT-specific content directly to polyserve.
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.
Updated 👍 I will update the WCT stuff accordingly in the other PR to minimize noise.
f7516a0
to
1c18a07
Compare
@@ -72,6 +74,9 @@ export function babelCompile( | |||
if (isPolyfill.test(request.url)) { | |||
return false; | |||
} | |||
if (shouldNotCompile.test(request.url)) { |
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.
Maybe use ('nocompile' in request.query)
or similar so that nocompile
doesn't have to be the first query parameter (e.g. foo&nocompile
).
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.
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.
Ah, TIL. Done 👍
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.
Add a changlog entry?
@aomarks Right, I always forget that. Have to add that PR_TEMPLATE soonTM 😉 |
By moving this enumeration from WCT to Polyserve, we can now make sure we do not compile these dependencies (they do not need it anyway).
Fixes #538