-
-
Notifications
You must be signed in to change notification settings - Fork 929
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
Remove createRuleTester #4267
Remove createRuleTester #4267
Comments
I use only |
Also some developers trying to use it and getting problems using it #4120 |
Exactly, it's way behind stylelint development and cause more harm.
We're going to release major version, so it's safe to remove. I'm thinking we could make The other solution, is to remove it from the export and deprecate (what @vankop did in #4265). This way we improve performance, and users (if any) would have more time to migrate from this API. But on the other side this code will stick with us until the next major release. It would create more work, that just remove this function, and give proper replacement in the coming month. I could take a lead on I'm very interested what people, who worked on stylelint the most, would say. /cc @jeddy3 @ntwb @evilebottnawi @m-allanson |
I'd like to see I quickly discovered lots of instances of
But it also did not take long before I discovered legit uses of |
last commit 3 years ago.. |
Ben's project is 3 year without updates. I found more alive projects: https://github.com/alexlafroscia/stylelint-plugin-css-modules/blob/master/test/helpers/test-rule.js How shall we deprecate it? If we just show deprecation message, then we won't gain any performance until the next major release. So 99.9% of our users (users, who don't use createRuleTester) won't get it. I would remove it completely. 10-20 people who might need it could wait few weeks. While tens of thousands users would benefit from performance improvements. Because we follow semver no builds would be broken. |
This example is only ~5 weeks ago: Personally I think it is poor form to deprecate anything without notice, we can follow up with another major release immediately after releasing v11.0.0, theoretically we could release v12.0.0 one minute after releasing v11.0.0. We can also make a note in the release notes that a v12 removing The v12 release does not require any other features or bug fixes to be included.
Deprecate it as per #4265 |
If we follow #4265 we don't need to create release as fast as possible (we remove |
While I was simplifying I didn't came up with a good solution :( Requirements:
Here's a tricky part: stylelint repository requires The only solution I came up with is dependency injection. In // jest-preset-stylelint
module.exports = function (stylelint) {
return function testRule(rule, schema) {
// here content of https://github.com/stylelint/stylelint/blob/master/jest-setup.js
describe(`${schema.ruleName}`, () => {
// ...
}
}
} In our repository we could consume it like: const testRule = require('jest-preset-stylelint');
const stylelint = require('./lib');
global.testRule = testRule(stylelint); In plugin repository it would be: const testRule = require('jest-preset-stylelint');
const stylelint = require('stylelint');
global.testRule = testRule(stylelint); Pros:
Cons:
ESLint has similar thing: Overall, I'm not sure if it worth it to create this shared Jest |
Two thumbs up on this
Would it be worth considering switching to a monorepo using Lerna? Hoisting dependencies might be a good alternative to dependency injection. We could consolidate all the stylelint repos into a single repo |
I used Lerna for almost a year at work. I wouldn't use it unless it's really needed. While it helps in situations with inter dependencies, but overall it makes things more complex. For users it's hard to find where the package, where is changelog (I always have hard time to find changelog from React team on things other than React itself). For developers Plus we would have cyclic dependency: |
We could expose Jest configuration as our Node.js package ( |
Hey guys, regarding this changes in the test environment for the stylelint ecosystem, how does this affects stylelint-test-rule-tape? I've been using it in stylelint-color-format and I wanted to know if I should move to a different approach, since stylelint-test-rule-tape uses the |
Ditto. I've copied the stylelint jest setup into too many plugin repos already. I'd love to see a The dependency injection route looks good to me.
It doesn't seem that often we need to update the test configuration. I think the overhead is worth it.
I believe |
If stylelint-test-rule-tape is now deprecated could you please update the docs and recommend an alternative solution? |
Shall we create and publish It would also be in line with having a one "blessed" editor integration (stylelint/vscode-stylelint). |
Looks like
createRuleTester
is an abandoned piece of API. We stopped use it almost three years ago.I've checked all plugins from https://stylelint.io/user-guide/plugins. None of them is using
createRuleTester
.I would consider
createRuleTester
even harmful to keep.createRuleTester
loads every syntax. And because it's a public API it happens every stylelint load.I propose to remove it. And then work on https://github.com/stylelint/jest-preset-stylelint, which stylelint itself could use for tests. And other plugins could use it as well.
The text was updated successfully, but these errors were encountered: