-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Publish separate package with rehype-minify-enumerated-attribute
schema?
#37
Comments
could you confirm whether the file currently matches what you’d expect? https://github.com/rehypejs/rehype-minify/blob/main/packages/rehype-minify-enumerated-attribute/schema.js Then I can publish it here as |
What I'd hope for is a common core package, that's used by one package for react jsx, and by another package for HAST. That way, changes that are jsx-only needn't cause churn here, and changes that are HAST-only needn't cause churn for eslint-plugin-react, but changes in the common core necessarily cause churn in both - and dependencies can be more granularly specified (so that fewer unnecessary things are installed). |
Hi folks, thanks for all the activity, ✨ sorry for being silent: I’ve just been taking this time to try and focus on some other OSS projects! I think we’re all pretty close to each other. I totes understand your last comment, Jordan! The HTML -> React or HTML -> hast mapping is rather long. Lots of unneeded complexity for the both of us. It may be nice for some other folks, but I’m doubting there’s that many folks interested in it! And: I don’t want to block this. I’ll get back to it soon, but for now: the schema is published already but as a file in a small package. And if any one wants to publish it (as React / HTML) casing, or copy-paste it into the code, that’s also fine with me! |
Hey @wooorm , would you still be open to publishing the |
added it, using HTML casing! |
Awesome! I don't see an "engines" field; can you confirm how far back it will work? eslint-plugin-react supports node 4+. |
ESM only 😬 |
@wooorm oof, that means it's incompatible with virtually everything and won't work at all for us. Any chance you could also provide a CJS implementation? |
Yeah, it’s a pickle. I’m afraid I’m rather hard headed on this one, I ported all my packages over to ESM, and am not providing dual (in my experience dual doesn’t work well in existing bundlers). Could you use an On the “that means it's incompatible with virtually everything” side, I disagree. 1% of all npm packages are ESM already! https://twitter.com/wooorm/status/1431289978961764357. A small number of course, but I think it signals that it’s going relatively fast. |
No, dynamic import doesn’t go anywhere near that far back - and eslint works async, so no part of the eslint ecosystem can ever use dynamic import, or native ESM - and the same applies to the babel ecosystem. |
You can of course do what you like, but it defeats the entire purpose of making separate packages if they can’t be maximally shared, and “esm only” guarantees they can’t be. So far what I've seen is that authors who move a package to pure ESM end up forcing their consumers to migrate off of it to a more compatible solution. |
While I completely understand that me making this ESM technically solves this issue but doesn’t solve the practical issue of making it work in eslint-plugin-react, I want to point that I’ve been fine with someone else publishing it, however they please, and me using it here, and want to reiterate that I still am!
This is turning into an ESM conversation, which I’m fine having and sharing my views on, but I also assume that you have good and thoughtful views on this. So if you don’t want to have this, that’s also fine! I understand that (gut) reaction but I’d add that in my experience: a) there are a few very vocal voices, I totally understand that it sucks that things change, but in my experience most people get over this phase, and alternatively the previous version is usable |
If I published the package as CJS, would you depend on it? The only benefit here is if we're both sharing the same data source. If not, then, if it were published as a dual package, would you depend on it? (it's simplest to keep it pure CJS, since ESM can import CJS) |
Both, yes. But for me to do it, what’s the time frame for you/eslint-plugin-react dropping CJS? As Node 4 is still supported, which was released 6 years ago and has been EOL for ages — I don’t want to maintain a CJS or dual package in 2027. |
In general, for all of my hundreds of packages, I don't ever drop support for anything unless there's a very strong reason - most of my packages still support node 0.4. Obviously if in 2027, nobody actually uses anything but ESM, then there'd be no benefit to supporting anything prior to that, but that's dictated by the community, and shouldn't be artificially forced by authors. I'm happy to maintain it, supporting node 4 (likely, 0.4, since it takes almost no effort to support everything), for as long as I'm alive - as long as you'd depend on it. I'll try to get something published in the next week. |
In my opinion ESM is one. Note that you can keep on maintaining previous versions, especially for security fixes.
I’d argue that you and I are part of the community and that there’s nothing artificial about using standard JavaScript features supported in all maintained JavaScript engines.
Awesome! |
I’ve published v0.1.0 and have added both of you as admins on npm: https://www.npmjs.com/package/html-enumerated-attributes! |
Adding "exports" is a breaking change, and this is a good reason to do a bump - but not a good reason to drop node versions, since dual packages do in fact work fine (if it breaks on a bundler, then don't use a broken bundler), and can be added semver-minor once "exports" exists. |
I appreciate the sentiment, and agree that it would be nice if build tools could smooth over more of this, create-react-app/webpack-4 does not remarkjs/react-markdown#518. And even if dual packaging did work well, it adds another burden on library maintainers.
I agree, but likely for very different reasons than you have. That said, I don't think libraries should go out of the way to break backwards compatibility either. |
Subject of the feature
The
rehype-minify-enumerated-attribute
schema looks super useful for things like minifiers and linters.Problem
I have recently done some research for a potential lint rule in
eslint-plugin-react
, and this schema looks like it would help a lot with the issue of "which attributes are already set to their default values?"Expected behavior
Separate package, similar to
html-element-attributes
Alternatives
Use another library, such as one from the list below:
html-minifier
htmlnano
The text was updated successfully, but these errors were encountered: