V8 Planned Changes

[bmish]

Starting a list of my desired breaking changes, feedback welcome. Likely this release won't happen until 2022-05 or 2022-06 when we can drop Node 12/17 support, unless we come up with additional breaking changes we want to release sooner.

• Drop support for Node 12, 14, 17, 19 #373
• Strictly define Node API #374
• Change require-expect rule default option to never-except-zero #375
• feat!: Require eslint@^8.38, drop support for eslint@7.x #376
• Upgrade dependencies in preparation for major release #377
• Enable additional recommended rules:
  • (no new rules to consider yet)

Changes for future major versions:

• Enable additional recommended rules:
  • New Rule: no-invalid-names #298

For reference, V7 was release 2021-10 (CHANGELOG, discussion in #175).

[sukima]

I would like to propose that the default require-expect rule be changed from except-simple to never-except-zero.

the use of expect was originally a legacy issue and has since been superseded with better patters like assert.async() and Promises. It would be helpful that teams who prefer the expect pattern can opt-in instead of continuing to recommend a pattern that is no longer needed and mostly redundant.

[bmish]

I agree that we shouldn't unnecessarily require expect everywhere. I also use the never-except-zero setting for require-expect in my projects.

[jacobq]

QUnit's docs for expect say:

It is recommended to test asynchronous code with assert.step() or assert.async() instead.

so I appreciate the move to "Change require-expect rule default to never-except-zero"
but I have a question about that. Do async tests still need to tell QUnit when they're done or does returning a promise (/ using an async function) do this automatically?

  // this is getting flagged by `require-expect` (technically correct but missing the point, I think)
  test('it does some async things just fine', async function (assert) {
    const done = assert.async(); // <== do I still need to do this? seems to work without it
    // ... do the testing ...
    done();
  })

In my case, the test is reading in a CSV file and performing an assert.strictEquals on every field, so there a zillions of them (and it seems silly/brittle to try to count them). Does it not make sense to have a default/recommended rule that doesn't seem to match QUnit's own recommendation or am I missing something?

[platinumazure]

@jacobq If you return a promise or use an async function, QUnit will wait for that promise to be resolved. So you shouldn't need to use assert.async in that case.

[jacobq]

@jacobq If you return a promise or use an async function, QUnit will wait for that promise to be resolved. So you shouldn't need to use assert.async in that case.

Thanks, that's sort-of what I thought (hadn't had to use async/done for a while), but I got confused about why the rule was telling me I needed expect. I've turned it off in my config now and am glad the default is getting changed.

[platinumazure]

@bmish This is becoming relatively urgent for us as GitHub Actions CI are now failing: https://github.com/platinumazure/eslint-plugin-qunit/actions/runs/5391205279/jobs/9787683963

Researched the issue and found this: https://stackoverflow.com/a/76255847

I think dependabot upgraded our lockfile to version 3 and that has broken our CI on npm 12 and 14. 12 is way past EOL and 14 recently hit EOL. So I think it's time to cut a major release here.

Is your list of proposed breaking changes still reasonably accurate? Do you have anything you want to add?

[bmish]

@platinumazure I agree it's a good time for a release. I just opened up PRs for the 3 breaking changes I would like to get included. If you want to review and approve those, then one of us can conduct the major release now (I'm happy to if that helps).