-
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
plugins/index.ts export should always be transpiled. #7051
Conversation
Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.
PR Review ChecklistIf any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'. User Experience
Functionality
Maintainability
Quality
Internal
|
@@ -180,7 +180,7 @@ module.exports = (ipc, pluginsFile, projectRoot) => { | |||
if (!tsRegistered) { | |||
try { | |||
const tsPath = resolve.sync('typescript', { | |||
basedir: this.projectRoot, |
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.
this.
is meaningless because it's an arrow function. It made projects load typescript from the cypress
package, not their project.
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.
yeah, between and project.js
and resolve.js
the duplication is hard to manage. Also, the tsnode options are better covered in #7166
0da640f
to
1cca0b6
Compare
I have confirmed that this PR finds the right user typescript module and transpiles the For testing, I suggest creating another fixture in packages/server/test/support/fixtures/projects and adding an e2e test in packages/server/test/e2e/3_plugins_spec.js |
9eb05e0
to
6cbdedf
Compare
I added a test that checks a project with This test makes me think:
|
Closing in favor of #7197 |
User facing changelog
When plugins/index.ts is not located at the default path and tsconfig.json has
"module": "esnext"
option, export is not transpiled. And it fails.Additional details
Why was this change necessary?
To make TypeScript support available even when cypress tests are not located at the default path.
What is affected by this change?
N/A
Any implementation details to explain?
Added
module: "CommonJS"
to the option.How has the user experience changed?
N/A
About test
Like #7072, this PR should also be tested on the master branch test repos. AFAIK, there is no way to test
cypress open --project test/e2e
command.PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?