developer-stylechain / gulp-stylelint

Gulp plugin for running Stylelint results through various reporters.
MIT License
14 stars 5 forks source link

Update to Stylelint 16 & Migrate to ESM #12

Open bpfoster opened 10 months ago

bpfoster commented 10 months ago

This builds on PR #11. My attempt to migrate gulp-stylelint to an ESM module.

onigoetz commented 9 months ago

Hi @ronilaukkarinen Can you please look into this PR?

Stylelint is a quite significant bump and this would be very valuable

thanks in advance

ronilaukkarinen commented 9 months ago

Hi @ronilaukkarinen Can you please look into this PR?

Stylelint is a quite significant bump and this would be very valuable

thanks in advance

Unfortunately I do not have time right now, we had to delay the introduction of Stylelint 16 with our team to Feb-Mar. We are currently using Stylelint 15. Feel free to test the PR yourself and review this, @bpfoster or @schalkneethling could perhaps help testing and merging this if they have more time in their hands?

ronilaukkarinen commented 5 months ago

Has @schalkneethling by any chance time to test this?

I'm still way too overbooked... We've been doing fine with stylelint 15 so far.

ronilaukkarinen commented 1 month ago

Finally took a look at this. Referencing issue https://github.com/developer-stylechain/gulp-stylelint/issues/10

Thank you for all the hard work @bpfoster. @kksandr7's point about https://github.com/adorade/gulp-stylelint-esm/issues/20 stands and that kinda woke me up, why we don't use gulp-stylelint-esm instead of gulp-stylelint? Doesn't it supersede this one?

Worth noting: I'm not 100% expert in advanced JS, just trying to keep this project alive as best I can.

When I tried the changes in this PR, I get:

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/rolle/Projects/gulp-stylelint/src/index.js from /Users/rolle/Projects/devpackages/gulp/tasks/lintstyles.js not supported.
Instead change the require of index.js in /Users/rolle/Projects/devpackages/gulp/tasks/lintstyles.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/rolle/Projects/devpackages/gulp/tasks/lintstyles.js:5:19) {
  code: 'ERR_REQUIRE_ESM'
}

I guess it is due to the breaking changes, so doesn't this mean README.md needs changes in instructions as well, you cannot use the Quick Start example as-is we have right now?

I'm not familiar with ESM and my current linstyles.js task is as follows:

// Dependencies
const {
  src
} = require('gulp');
const stylelint = require('@ronilaukkarinen/gulp-stylelint');
const config = require('../config.js');

// Task
function lintstyles() {

  return src([config.stylelint.src])

    // Print linter report
    .pipe(stylelint(config.stylelint.opts));
}

exports.lintstyles = lintstyles;

What exactly needs to be changed?

Also:

npm error Could not resolve dependency:
npm error peer stylelint@"10 - 15" from @ronilaukkarinen/stylelint-a11y@1.2.9
npm error node_modules/@ronilaukkarinen/stylelint-a11y
npm error   dev @ronilaukkarinen/stylelint-a11y@"^1.2.7" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.

But as it seems there is another actively developed stylelint-a11y package that has 16 support so we can go around that and leave our stylelint-a11y as-is.