Skip to content

Update to Stylelint 16 & Migrate to ESM#12

Open
bpfoster wants to merge 2 commits intodeveloper-stylechain:masterfrom
bpfoster:gh-issue10-esm
Open

Update to Stylelint 16 & Migrate to ESM#12
bpfoster wants to merge 2 commits intodeveloper-stylechain:masterfrom
bpfoster:gh-issue10-esm

Conversation

@bpfoster
Copy link

@bpfoster bpfoster commented Jan 3, 2024

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

Stylelint 16 had quite a few changes
This will update gulp-stylelint to a minimally-working state with v16.
Note that CommonJS is deprecated in stylelint 16.  This does not update gulp-stylelint to ESM, but that will need to happen before v7

fixes developer-stylechain#10
@onigoetz
Copy link

onigoetz commented Feb 2, 2024

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
Copy link
Member

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
Copy link
Member

ronilaukkarinen commented May 10, 2024

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.

*
* @type {Function}
*/
const formatter = typeof config.formatter === 'string' ?
Copy link

@kksandr7 kksandr7 Jun 14, 2024

Choose a reason for hiding this comment

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

I think there will be the same problem as here adorade/gulp-stylelint-esm#20

@ronilaukkarinen ronilaukkarinen changed the title Update to Sylelint 16 & Migrate to ESM Update to Stylelint 16 & Migrate to ESM Sep 6, 2024
@ronilaukkarinen
Copy link
Member

ronilaukkarinen commented Sep 6, 2024

Finally took a look at this. Referencing issue #10

Thank you for all the hard work @bpfoster. @kksandr7's point about adorade/gulp-stylelint-esm#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.

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.

4 participants