-
-
Notifications
You must be signed in to change notification settings - Fork 724
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: handle importing custom helper nicely #3691
Conversation
@@ -165,7 +165,11 @@ function createHelpers(config) { | |||
} else { | |||
moduleName = `./helper/${helperName}`; // built-in helper | |||
} | |||
const HelperClass = require(moduleName); | |||
|
|||
// @ts-ignore |
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.
Could we avoid of the // @ts-ignore
by something like this:
const getModule = (moduleName) => {
const module = require(moduleName);
return module.default ? module : { default: module };
};
const isHelperModule = moduleName.startsWith('./helper/');
const HelperClass = isHelperModule
? require(moduleName)
: getModule(moduleName).default;
?
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.
Oh... I see. Hard situation.
A good time to think about switching to TypeScript and solve the typing issue the correct way 🙂
Sorry, I can't see purpose of this
Moving away from CommonJS would be a nice idea but I see that it requires a lot of time, especially as we have dynamic imports everywhere. |
Hey @DavertMik, I wanted to discuss a change regarding how we handle the loading of custom helpers. Nowadays, most users are using TypeScript for their custom helpers, and the "export =" syntax, which is deprecated, is being replaced with "export default" to export classes. However, this change causes errors when users try to do the same with CodeceptJS. This pull request aims to address this issue by ensuring that any syntax used to export classes in custom helpers will not cause any problems. By doing so, we won't limit users in their custom helper choices. Of course, it's important to note that their custom helpers will remain their responsibility, but our core functionality will continue to work flawlessly. This change could be a way to attract more users. |
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.
OK, I'm convinced to accept this
But please do a minor change to improve the code readability
Motivation/Description of the PR
Type of change
Checklist:
npm run docs
)npm run lint
)npm test
)