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

Ensure "type" = "module" ES declaration in pre-release.sh #4350

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

BLCK-B
Copy link
Contributor

@BLCK-B BLCK-B commented Aug 15, 2024

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Attempts to fix the issue #4347. Added a line in scripts/pre-release.sh that should add "type" = "module" to package.json. Analogous to 432aaa4.

Reasoning:
CI order goes Release Process > Release Make > pre-release.sh. It does not involve the other release script with the previous fix.

Unfortunately, I can't test or verify. The CI workflows are too complex.

Signed-off-by: Jonas Black [email protected]

@BLCK-B BLCK-B requested a review from a team as a code owner August 15, 2024 14:26
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label Aug 15, 2024
@BLCK-B BLCK-B changed the title Add "type" = "module" to ensure it is present Ensure "type" = "module" ES declaration in pre-release.sh Aug 15, 2024
Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

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

I'm surprised to learn that matrix-js-sdk is being released as an ES module nowadays, because I thought matrix-react-sdk couldn't consume those yet. But, this fix seems appropriate especially with the context from Rich in the linked issue. Thanks!

@robintown robintown added this pull request to the merge queue Aug 15, 2024
Merged via the queue into matrix-org:develop with commit d608039 Aug 15, 2024
33 of 36 checks passed
@dbkr
Copy link
Member

dbkr commented Aug 20, 2024

Sorry, this appears to be breaking our release process https://github.com/matrix-org/matrix-js-sdk/actions/runs/10469900988/job/28993898692 and is preventing me from doing a release. I'm going to have to revert this.

dbkr added a commit that referenced this pull request Aug 20, 2024
github-merge-queue bot pushed a commit that referenced this pull request Aug 20, 2024
richvdh added a commit that referenced this pull request Aug 20, 2024
github-merge-queue bot pushed a commit that referenced this pull request Aug 20, 2024
* Reapply "Add "type" = "module" to ensure it is present (#4350)" (#4352)

This reverts commit 8214fd7.

* Mark prettier config file as CommonJS

I *think* this will fix a problem with the release process in which we saw an
error:

```
Error:  Invalid configuration for file "/home/runner/work/matrix-js-sdk/matrix-js-sdk/package.json":
Error:  module is not defined in ES module scope
Error:  This file is being treated as an ES module because it has a '.js' file extension and '/home/runner/work/matrix-js-sdk/matrix-js-sdk/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
```
@richvdh richvdh linked an issue Aug 20, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Defect Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use import statement outside a module (ES, CommonJS)
3 participants