feat!: migrate to pure ESM #3850

JounQin · 2024-01-12T16:48:25Z

Description

As title

Motivation and Context

close #3642
close #3705
close #3842

Usage examples

N/A

How Has This Been Tested?

Yes

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 change)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

TODO list after merged:

add changesets and related documentations
update more ESM packages
change to use exports field in package.json (via @marcalexiei)

JounQin · 2024-01-12T17:08:48Z

I haven't upgraded all dependencies to pure ESM like execa, I'd like to upgrade these legacy dependencies in another PR.

escapedcat · 2024-01-15T08:38:39Z

Thanks @JounQin !

JounQin · 2024-01-15T14:10:24Z

@escapedcat I'm not for sure to understand this failing test case. Would you like to help?

JounQin · 2024-01-15T14:43:39Z

Interesting unexpected change here.

conventional-changelog-angular does not support fix(scope)!: actually.

Founded and fixed at 45b6f78 (#3850)

JounQin · 2024-01-15T15:48:20Z

The last failing test case I've just found is that jest.mock nor jest.unstable_mockModule does not work for import() yet.

See also jestjs/jest#14856

JounQin · 2024-01-15T17:17:41Z

I'll try to migrate to vitest tomorrow.

ZynthCode · 2024-01-26T08:28:27Z

I'll try to migrate to vitest tomorrow.

How did it go?

JounQin · 2024-01-26T08:47:19Z

I'll try to migrate to vitest tomorrow.

How did it go?

Oh, I'm sorry, I was a bit business busy these days and totally forgot this PR. I'll find some time to work on this soon later.

JounQin · 2024-01-27T23:58:41Z

@escapedcat I've rebased from master branch, CI is required to be approved.

JounQin · 2024-01-28T16:12:34Z

CI is required to be approved again. 🤣

JounQin · 2024-01-29T03:02:37Z

@escapedcat I've rebased from master branch, CI is required to be approved.

😅

marcalexiei

Hello @JounQin,
thank you for your work in this migration!

I noticed a few things that might be changed in order to avoid additional workflow approval 😂

.github/workflows/CI.yml

license.md

@commitlint/rules/src/header-trim.ts

JounQin · 2024-01-29T08:56:55Z

@escapedcat @marcalexiei Would you like to help to fix these test cases?

https://github.com/JounQin/commitlint/blob/a36a87dbaa14d67a09be1303485db57242fbac9b/%40commitlint/load/src/load.ts#L30-L61

marcalexiei · 2024-01-29T12:59:31Z

Hey @JounQin,
I never worked on these specific tests but I’ll try to take a look in the next days.

marcalexiei

@JounQin I tackled with the tests and I found a possible cause of the issue.
Does it make sense to you?

I have also added some additional thoughts.

auto-imports.d.ts

@commitlint/load/src/load.ts

JounQin · 2024-01-30T12:56:49Z

👍 I'll take a look when I'm free enough.

escapedcat · 2024-02-19T10:54:33Z

Should these be updated as well then?

  "engines": {
    "node": ">=v18"
  },

JounQin · 2024-02-19T10:59:48Z

I think it's good to only support LTS versions.

knocte · 2024-02-19T11:33:07Z

I think it's good to only support LTS versions.

Ubuntu 24.04 is LTS and has Node 18. Please revert Node version back to 18.

escapedcat · 2024-02-19T11:36:57Z

Soo...
These pure "ESM only" tests fail with v18. This whole PR is a breaking change anyway.
How to deal with this?

We keep 18 but tests will be done in 20?

knocte · 2024-02-19T11:40:03Z

These pure "ESM only" tests fail with v18.

Why do you say that? CI was green.

escapedcat · 2024-02-19T11:51:27Z

Sorry for the confusion. Reverted it. Somehow I got confused by the locally failing tests.
Happy Monday! :P

escapedcat · 2024-02-20T13:16:43Z

Ping me if this can be published. Will wait with all other/unrelated PRs till then. No rush.

BREAKING CHANGE: migrate to pure ESM * feat: migrate to pure ESM * chore: update snapshot * fix: load `parserPreset` with another `await` * test: migrate to vitest * test: remove no replacement `--runInBand` test-ci script * chore: fix code reviews * refactor(load): rewrite resolve logic * fix(config-nx-scopes): fix syntax error * feat(resolve-extends): add resolveFrom and loadParserPreset * feat(load): use resolveFrom and loadParserPreset from resolve-extends * test: include only @commitlint/* packages src in coverage * test: explicit import vitest utilities * test: remove @jest/globals from dependencies * fix(resolve-extends): `resolveFrom` output should be platform aware * test: restore NO_COLOR to test script * chore: fix linting issues * fix: should use fileURLToPath instead of pathname for Windows compatibility * Apply suggestions from code review * fix: should reuse `cli` instead call `yargs()` * feat(cli): set terminalWidth as wrap to avoid work break on help * Update .eslintrc.cjs * feat: migrate @commitlint/config-conventional to pure ESM --------- Co-authored-by: Marco Pasqualetti <[email protected]>

escapedcat · 2024-02-27T11:17:55Z

https://github.com/conventional-changelog/commitlint/releases/tag/v19.0.0

escapedcat · 2024-02-27T11:18:11Z

Thanks everyone! Please give it a try

escapedcat · 2024-02-27T14:11:01Z

Any idea?
#3936

JounQin · 2024-02-27T14:28:07Z

Any idea?

#3936

@escapedcat I'll take a look if reproduction provided.

CJKay · 2024-03-07T16:09:26Z

We are still seeing the behaviour described by #3949 on 19.0.0 and 19.0.3. 😞

JounQin · 2024-03-08T02:13:31Z

@CJKay commitizen is not controlled by commitlint, if you're not ready for pure ESM commitlint, don't upgrade.

frantic1048 · 2024-03-08T02:14:02Z

@CJKay, because commitizen does not support ESM (commitizen/cz-cli#916), issues are expected to arise when @commitlint/cz-commitlint becomes an ESM module.

A temporary workaround, as described in #3949 (comment), involves wrapping @commitlint/cz-commitlint into a CJS module and allowing commitizen to use the wrapped module as an adapter under every developer's project.

The long-term fix is to add ESM support in commitizen. There are no fixes required on the side of @commitlint/cz-commitlint.

JounQin · 2024-03-08T02:22:23Z

@frantic1048 Considering the workaround, I have an idea to make @commitlint/cz-commitlint into a dual package, do you want to raise a PR? If not, I'll do it myself.

frantic1048 · 2024-03-08T02:27:39Z

@JounQin

Given the facts that:

@commitlint/cz-commitlint is intended for use with commitizen
commitizen currently does not support ESM, and this may persist given that their issue supporting ESM was raised two years ago

It follows that the latest @commitlint/cz-commitlint will not work because no version of commitizen supports ESM.

I'm not opposing the ESM-only or any particular module format; however, could we address the usability issue for @commitlint/cz-commitlint specifically (not entire @commitlint scope) by providing an alternate CJS export?

JounQin · 2024-03-08T02:29:45Z

@frantic1048 Yes, that's what I sad to make it a dual package, do you want to raise a PR?

frantic1048 · 2024-03-08T02:32:12Z

@JounQin, yes, allow me some time to delve into the codebase.

JounQin · 2024-03-08T02:35:53Z

@frantic1048 It's just easy to add a src/index.cts entry and wrap await import('./index.js'), and add an exports field into package.json.

frantic1048 · 2024-03-08T03:29:04Z

@JounQin, I have proposed a PR that adds an CJS export for @commitlint/cz-commitlint and have tested it in my own repositories; it works fine. Please take a look. 👀

#3963

CJKay · 2024-03-08T13:10:17Z

@JounQin @escapedcat Right-o, sorry, these errors threw me for a bit of a loop - I'm not particularly familiar with the JavaScript ecosystem.

* feat: add CJS export for cz-commitlint relates to: #3850 (comment) resolves #3949 * chore: add .cts to eslintrc * fix: workaround cts issues of vitest and typescript

JounQin force-pushed the feat/esm branch 5 times, most recently from 2dd7414 to 74b4111 Compare January 12, 2024 17:06

JounQin mentioned this pull request Jan 12, 2024

fix: use ESM for cli #3642

Closed

4 tasks

JounQin changed the title ~~feat: migrate to pure ESM~~ feat!: migrate to pure ESM Jan 12, 2024

JounQin force-pushed the feat/esm branch 2 times, most recently from 1055feb to f7b0772 Compare January 15, 2024 13:02

JounQin mentioned this pull request Jan 15, 2024

[Bug]: jest.unstable_mockModule does not work with custom dynamic import wrapper jestjs/jest#14856

Open

escapedcat mentioned this pull request Jan 26, 2024

feat!: move from CommonJS to ESM #3705

Closed

JounQin force-pushed the feat/esm branch from 65883e0 to f626df7 Compare January 27, 2024 19:51

JounQin force-pushed the feat/esm branch from 921621e to cb8a2a1 Compare January 28, 2024 16:09

JounQin force-pushed the feat/esm branch from cb8a2a1 to b34bf9d Compare January 29, 2024 03:00

marcalexiei reviewed Jan 29, 2024

View reviewed changes

.github/workflows/CI.yml Show resolved Hide resolved

license.md Outdated Show resolved Hide resolved

@commitlint/rules/src/header-trim.ts Outdated Show resolved Hide resolved

marcalexiei reviewed Jan 30, 2024

View reviewed changes

auto-imports.d.ts Outdated Show resolved Hide resolved

@commitlint/load/src/load.ts Outdated Show resolved Hide resolved

Zhengqbbb mentioned this pull request Feb 29, 2024

[Bug report] 使用@commitlint/config-conventional^19出现异常 Zhengqbbb/cz-git#160

Closed

2 tasks

JounQin mentioned this pull request Feb 29, 2024

Fix false positive CJS deprecation warning for dual-package plugins stylelint/stylelint#7532

Merged

Zhengqbbb mentioned this pull request Mar 3, 2024

support commitlint v19 esm plugin extends config Zhengqbbb/cz-git#161

Merged

6 tasks

escapedcat mentioned this pull request Mar 4, 2024

Can not use @commitlint/cz-commitlint as commitizen's adapter #3949

Closed

4 tasks

frantic1048 mentioned this pull request Mar 8, 2024

feat: add CJS export for cz-commitlint #3963

Merged

7 tasks

galtm mentioned this pull request Mar 12, 2024

ci(deps-dev): bump commitlint from 18.6.1 to 19.1.0 xspec/xspec#1871

Merged

feat!: migrate to pure ESM #3850

feat!: migrate to pure ESM #3850

Conversation

JounQin commented Jan 12, 2024 • edited Loading

Description

Motivation and Context

Usage examples

How Has This Been Tested?

Types of changes

Checklist:

JounQin commented Jan 12, 2024

escapedcat commented Jan 15, 2024

JounQin commented Jan 15, 2024

JounQin commented Jan 15, 2024 • edited Loading

JounQin commented Jan 15, 2024 • edited Loading

JounQin commented Jan 15, 2024

ZynthCode commented Jan 26, 2024

JounQin commented Jan 26, 2024 • edited Loading

JounQin commented Jan 27, 2024

JounQin commented Jan 28, 2024

JounQin commented Jan 29, 2024

marcalexiei left a comment

Choose a reason for hiding this comment

JounQin commented Jan 29, 2024

marcalexiei commented Jan 29, 2024

marcalexiei left a comment

Choose a reason for hiding this comment

JounQin commented Jan 30, 2024

escapedcat commented Feb 19, 2024

JounQin commented Feb 19, 2024

knocte commented Feb 19, 2024

escapedcat commented Feb 19, 2024 • edited Loading

knocte commented Feb 19, 2024

escapedcat commented Feb 19, 2024

escapedcat commented Feb 20, 2024

escapedcat commented Feb 27, 2024

escapedcat commented Feb 27, 2024

escapedcat commented Feb 27, 2024

JounQin commented Feb 27, 2024 • edited Loading

CJKay commented Mar 7, 2024

JounQin commented Mar 8, 2024

frantic1048 commented Mar 8, 2024

JounQin commented Mar 8, 2024

frantic1048 commented Mar 8, 2024

JounQin commented Mar 8, 2024

frantic1048 commented Mar 8, 2024

JounQin commented Mar 8, 2024 • edited Loading

frantic1048 commented Mar 8, 2024

CJKay commented Mar 8, 2024 • edited Loading

JounQin commented Jan 12, 2024 •

edited

Loading

JounQin commented Jan 15, 2024 •

edited

Loading

JounQin commented Jan 15, 2024 •

edited

Loading

JounQin commented Jan 26, 2024 •

edited

Loading

escapedcat commented Feb 19, 2024 •

edited

Loading

JounQin commented Feb 27, 2024 •

edited

Loading

JounQin commented Mar 8, 2024 •

edited

Loading

CJKay commented Mar 8, 2024 •

edited

Loading