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

Remove dependency on node:util and window #1752

Merged
merged 7 commits into from
Sep 15, 2021

Conversation

aslakhellesoy
Copy link
Contributor

@aslakhellesoy aslakhellesoy commented Sep 14, 2021

Summary

Remove dependency on Node.js and Browser APIs

Details

We were depending on util in a couple of places:

  • util.deprecate was used to deprecate the CucumberExpressionGenerator#generateExpression() method.
  • util.format was used to build generated Cucumber expressions (from undefined steps).

We were depending on window in a file that wasn't used or exposed in the exports.

All of this has been removed. In the process the deprecated CucumberExpressionGenerator#generateExpression() method has also been removed. For consistency the Ruby CucumberExpressionGenerator#generate_expression method has also been removed. No changes were needed in the Go and Java implementation (the deprecated method was not present).

Finally, a couple of obsolete module.exports assignments have also been removed.

Motivation and Context

The use of the Node.js util module causes a build error when esbuild is used to build a project that depends on this library:

 > node_modules/@cucumber/cucumber-expressions/dist/src/GeneratedExpression.js:6:39: error: Could not resolve "util" (use "--platform=node" when building for node)
    6 │ const util_1 = __importDefault(require("util"));
      ╵                                        ~~~~~~

 > node_modules/@cucumber/cucumber-expressions/dist/src/CucumberExpressionGenerator.js:8:39: error: Could not resolve "util" (use "--platform=node" when building for node)
    8 │ const util_1 = __importDefault(require("util"));
      ╵                                        ~~~~~~

How Has This Been Tested?

With Mocha :-)

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist:

  • The change has been ported to Java.
  • The change has been ported to Ruby.
  • The change has been ported to JavaScript.
  • The change has been ported to Go.
  • The change has been ported to .NET.
  • I've added tests for my code.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have updated the CHANGELOG accordingly.

@aslakhellesoy aslakhellesoy changed the title Remove deprecated generateExpression method Remove dependency on node:util Sep 14, 2021
@aslakhellesoy aslakhellesoy changed the title Remove dependency on node:util Remove dependency on node:util and window Sep 14, 2021
@aslakhellesoy aslakhellesoy added 🐛 bug Defect / Bug 🔧 build Related to build / release process labels Sep 14, 2021
@@ -11,10 +11,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed

* Remove dependency on Node.js APIs (`util` module)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Remove dependency on Node.js APIs (`util` module)
* Remove dependency on Node.js APIs (`util` module)
([#1752](https://github.com/cucumber/common/pull/1752))

cucumber-expressions/CHANGELOG.md Show resolved Hide resolved
cucumber-expressions/CHANGELOG.md Show resolved Hide resolved
@aslakhellesoy aslakhellesoy merged commit 208800c into main Sep 15, 2021
@aslakhellesoy aslakhellesoy deleted the cucumber-expressions-remove-generate-expression branch September 15, 2021 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Defect / Bug 🔧 build Related to build / release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants