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

Parse compiled cjs step definitions #222

Conversation

finnmerlett
Copy link
Contributor

@finnmerlett finnmerlett commented Jul 11, 2024

🤔 What's changed?

Updated the javascript defineStepDefinitionQueries array to include a Tree-sitter query that matchs steps in the format found in compiled commonjs files.

Javascript steps in the following format will now be identified:

;(0, cucumber_import.Given)('a compiled format', function () {
  ...
})

NOTE: The function used for the second argument is completely ignored by the query - it makes no difference if it is async or not, the step will still be matched.

⚡️ What's your motivation?

Fixes #187 - allows steps to be identified in compiled cjs/js files, usually needed when importing from other modules (aka in node_modules)

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

  • Are the doc changes required/suitable?

📋 Checklist:

[ ] My change requires a change to the documentation (potentially, not crucial though)

  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@finnmerlett finnmerlett changed the title Expand javascript step definition queries to match compiled cjs style… Expand js step definition queries to match compiled cjs style steps Jul 11, 2024
@kieran-ryan kieran-ryan added 🐛 bug Defect / Bug ⚡ enhancement Request for new functionality and removed 🐛 bug Defect / Bug labels Jul 14, 2024
Copy link
Member

@kieran-ryan kieran-ryan left a comment

Choose a reason for hiding this comment

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

Awesome contribution @finnmerlett! Extended the query to support direct imports with this step definition format also (i.e. Given rather than cucumber.Given)

@kieran-ryan kieran-ryan changed the title Expand js step definition queries to match compiled cjs style steps Compiled cjs step definitions Jul 14, 2024
@kieran-ryan kieran-ryan merged commit b452811 into cucumber:main Jul 14, 2024
3 checks passed
@kieran-ryan kieran-ryan changed the title Compiled cjs step definitions Parse compiled cjs step definitions Jul 14, 2024
@finnmerlett
Copy link
Contributor Author

finnmerlett commented Jul 16, 2024

@kieran-ryan

Awesome contribution @finnmerlett! Extended the query to support direct imports with this step definition format also (i.e. Given rather than cucumber.Given)

Thanks for the merge 💪 good to see the fix make its way in! Just FYI in terms of the edit you made, doesn't do any harm but probably isn't needed, since in cjs compiled format where you have (0, import.property)(...) it is likely always a property access and never a direct import usage.

If you're interested, the reason (which I only found out when I was looking stuff up for this fix) that you find this weird syntax in compiled cjs files is because it is a quick way to ensure the this binding inside the property is set to the global scope (aka undefined in strict-mode). By returning the property accessor from an immediate expression evaluation aka (0, import.property), you detach it from its object without having to assign it to an intermediary variable.

The global-this-context'ed usage of member imports is what the esm import style import { method } from 'package' does automatically, but since commonjs uses require() this hack is the standard shorthand to mimic the behaviour.

Unless of course you found the (0, method) syntax in a published package somewhere, in which case I stand corrected haha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to locate Javascript steps generated from building Typescript files
2 participants