Skip to content

Add the commonjs_server option to the javascript_server outputter#726

Merged
akkomar merged 1 commit intomozilla:mainfrom
chenba:backend-commonjs
Jul 15, 2024
Merged

Add the commonjs_server option to the javascript_server outputter#726
akkomar merged 1 commit intomozilla:mainfrom
chenba:backend-commonjs

Conversation

@chenba
Copy link
Contributor

@chenba chenba commented Jul 12, 2024

Unfortunately some nodejs services still need to use CommonJS. This patch adds an -f option to generate JS in the CommonJS module format.

Pull Request checklist

  • Quality: This PR builds and tests run cleanly
    • make test runs without emitting any warnings
    • make lint runs without emitting any errors
  • Tests: This PR includes thorough tests or an explanation of why it does not
    • I didn't see any existing tests for the javascript_server outputter that tests individual -f options...?
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to language binding APIs are noted explicitly

@chenba
Copy link
Contributor Author

chenba commented Jul 12, 2024

@akkomar can you take a look? Thanks!

@badboy badboy requested a review from akkomar July 15, 2024 08:45
Copy link
Collaborator

@akkomar akkomar left a comment

Choose a reason for hiding this comment

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

@chenba for consistency with rest of the code here I'd make commonjs an option of javascript_server outputter. I put up a change in this PR to your fork: chenba#1

@chenba chenba requested a review from a team as a code owner July 15, 2024 13:08
@chenba chenba requested review from chutten and removed request for a team July 15, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants