-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Consider explicit string formatting module #43382
Comments
What's the motivation for this? Why does util need to have color formatting and all of these other bells and whistles, taking an opinionated stance on these things? Say what you want about Marak, but don't pin that on other packages that do ANSI colors, please. Your PR is just moving well established ecosystem packages into Node core for... no apparent reason. Not only is this unnecessarily bloating Node's util module, but you're kind of disregarding a lot of work over the last decade. And to be completely honest, it's a bit insulting you copied almost entirely the Chalk API, seemingly without any concern for how the maintainers would feel about it. |
It’s not for no reason; most of the world uses CJS and will for the foreseeable future, and there’s no longer a maintained package available that works with it - and node often moves packages into core, as does the language - that’s the goal of a platform. A package is successful when it’s obsoleted by the platform. |
I too welcome that funcionality like ANSI colors is moving into core. Every dependency is a potential security risk waiting to happen. Besides that, reducing the number of dependencies is also good for performance and stability. |
I have no problem vendoring in an ecosystem module as part of the vendor namespace idea. I'm not convinced it makes sense to bake directly into the core api. |
@Qix- what would be a better way to interact assuming there is merit in this functionality in core? Core cares a bunch about the community, the ecosystem and maintainers like you. As someone who maintained several libraries whose functionality was subsumed by core I can totally see how this isn't clear-cut and it's nice/flattering but also frustrating. The best case scenario for Node.js would be if you helped maintain and consult on these APIs in core (if Node decides this belongs in core, which can also be through vendoring a blessed module under the node namespace) which would ideally enable you to keep benefiting from them on one hand (for example through sponsorhsips) but would also simplify things for users. I do see the tension here and I acknowledge core has a lot of power over the ecosystem, the ecosystem is really important to Node's success and the last thing most team members I talked to about this want to do is hurt the authors. (I will not comment about Marak since our policy is not to discuss specific people and moderation in public) I am not sure what the best scenario for you is and what would work well for you - so please let me know. Knowing @BridgeAR I don't think there was any intention to insult you (but Ruben can weigh in), the fact he liked the chalk API so much he picked it (or something similar) is a sign of respect (similarly to how you maintaining a library that allows debugging using Node's internal API is not seen as bad in any way). On the other hand, I would ask core members (and specifically @BridgeAR who is working on the PR) not to land this until good-faith discussion with @Qix- has exhausted itself and we've both listened to what they have to say and reached consensus amongst ourselves. I know this is common practice in Node anyway but I just want to explicitly point it out. |
Here's my take. If Node.js is moving popular modules out of the ecosystem into core for the sake of API compatibility...
The original post's template seems to ask "what problem with this solve?" More contextually, "what problem is solved by invalidating Chalk from the ecosystem as a long-standing and well trusted package, and moving it into core with the exact same API and functionality? Perhaps this seems snarky, but this is precisely what the question is getting at.
I don't see how this answers the question at all. What is the point of pulling these modules into core and inflating the standard library? This question remains unanswered. Admittedly, I had no idea that node/lib/internal/util/inspect.js Lines 224 to 232 in 027c288
Yep. That regex pattern that we spent collective days refining, testing, and hardening. Vendored into node, without notifying the maintainers (again, @sindresorhus correct me if I'm wrong - I certainly wasn't notified). This regex has had ReDos vulnerabilities reported for it before. How does Node intend to maintain this regex? What was the point of pulling this into core? What is the security benefit of increasing vulnerability surface area? I'm not here to have the license battle because I'll lose that. That's what the license is there for. I'm happy we as a community have defaulted to something like the MIT. I have no problem with anyone vendoring or privately using my code in such cases. This still feels quite slimy, because the very platform we invested our time in has now absorbed our hard work into core and completely invalidated any reason for people to refer to our package and to receive updates and to report to us any issues they find to get support and fixes quickly and directly. The Node core team has unilaterally agreed, then, that the open source model for their own ecosystem does not work. I would say this is an oversight in thought processes within the committee, but there's already precedent here with Though the fact there is precedent here does not mean it's alright. It feels like the Node committee is trying to EEE us - pretty deliberately. I had to make sure that Node wasn't also acquired somehow by Microsoft when writing this, and it turns out that the OpenJS foundation is owned by a bunch of corporations, including Microsoft. The merger of the Node.js foundation into OpenJS happened in 2019. Node 16.11 - the version where
This makes little sense; I have questions.
I don't want to pull this card very often but this is one of the few places I will. I've been a diligent, happy proponent of Node.js since 0.10 was released. I've been a maintainer of several top-10 packages for almost, if not over, 10 years at this point. I've brought Node.js to many companies, supported and contributed to countless repositories on GitHub for over a decade now, I've even contributed directly to libuv and Node.js. If this gets merged, I will consider myself directly and forcibly pushed out of the Node.js community by the Node and OpenJS committees. I'm not sure how much you actually care about this, but for me, as having attributed a large percentage of my career's time and success to this ecosystem and language, it will be the end of an era. Maybe I'm overreacting. But this feels wrong. |
I intentionally opened the PR as draft and I reached out to @sindresorhus to let them know about it when I did that. Using the same API as chalk is indeed a form of great respect for the work that has been done as being proven to be intuitive and good. I think it would be great to get the chance to discuss the general that has now come up due to me opening the draft for adding further coloring functionality to core in more depth. There was definitely no intention to do any harm when it was brought up at the collaborator summit to add more coloring functionality to core. @Qix- I agree that things could have gone better. I hope we can get back to good terms as I personally hugely appreciate your great work in the ecosystem! |
I am going to close the Draft for now until we are able to resolve this topic. |
I would like to add a few fundamental points. Should Node.js produce output that has colors?My answer to this question is a strong yes, as everybody likes colors in their terminal.
I would really like to know @Qix- opinion on this topic. I think solving this would shape Node.js future. ESM migrationMost of the real-world team I have worked in the last two-three years have no plan to migrate to ESM as there is no business benefit for them. Very few managers would allocate budget to the migration: it's significantly simpler to switch to a dependency that still support CJS. The transition to ESM is taking significantly more time than what others have expected. ESM is still lacking a few fundamental features that makes CJS great. Very few companies or individuals are willing to invest the time and effort needed to fill the gap. The download numbers of chalk and other libraries tells the same story: most users are not upgrading to ESM, as most people are staying on an old version which is a great solution short term. However, I don't expect to migrate any of my code to ESM-only anytime soon, therefore I had to seek an alternative. The chalk team decided that it was not interesting for them to support my use case: this is their decision to make. This decision to go ESM-only created significant issues in the community and a high number of people has asked me what to do. What can Node.js core do? This functionality is a really good candidate for inclusion in core, because almost everybody wants it, include core itself. Core vs EcosystemI don't see a conflict with bundling some feature in core vs have it in the ecosystem. Existing, proven technologies are extremely hard to displace. As an example, jQuery is still in used by the vast majority of websites. The ecosystem can move at a speed of innovation that can be hardly matched by a project that has a LTS cycle of 3 years. I don't think chalk or any other library would be impacted at all. Governance of Node.jsNode.js is governed by its collaborators, not by "Big Tech". The best way to have a say in these decision is to... contribute. People are willing to contribute new features to core: should the collaborators block them or facilitate them? |
Thanks for engaging @Qix- , Node.js is a distributed project and I only represent one person in it - a maintainer but not in a leadership position like Matteo or Ruben above. I've still been involved long enough to know maintainers typically have a say here. Both these people above (Matteo and Ruben) are ecosystem maintainers themselves. I'm an ecosystem maintainer whose work was integrated into core before - I believe we do have some perspective. I also strongly believe your involvement is a net-positive for all parties here. You raise some good points .
I've had a module I maintain (bluebird) integrated into core in the past. A lot of the ideas we've had there like long stack traces, better debuggability as well as performance (V8 uses the bluebird benchmark themselves) were upstreamed into core. I think a big part of the difference in my experience from yours is that I was a part of most of it and so was Petka (the original author).
I think the project's communication here did leave a lot to be desired. It is now clear (following Ruben's message) there was an intention to ping maintainers for feedback before merging it but it looks like that wasn't communicated well initially. I think this is regretful. I think there was no intention to harm chalk and as you can (hopefully) see from member responses after your initial comment: people understand we've made a mistake here and we'd like to improve in good faith. I can't apologize on behalf of the whole project - but I will as myself: I'm sorry about the project taking external work from the ecosystem without attempting to contact maintainers, even though this was a draft PR the fact maintainers were to be contacted should have been made explicit and done sooner and you shouldn't have found out about this from outside the project. We've done better in the past and I will personally ping maintainers when I see this in the future. Moreover, I am sorry the project to my knowledge didn't consider the implications on the maintainers of that work before. This was a mistake and didn't give enough space to the support the project has received from the ecosystem and how fundamental it was and is to its success. I vouch to personally call it into attention when I see these issues in the future. That said, I also don't consider this theft. I do believe the fact the functionality provided is now (possibly) considered important, common and fundamental enough to be in core is a good thing. I believe we have to think more about how we celebrate the hard work put into these pieces of code.
I know this doesn't make up for past miscommunications but let me personally invite you to attend the next collaborator summits (in person or virtually). If you'd like to participate in or host a meeting about colors in terminals that could be a great win. To answer some of the questions I can:
Generally no. TC39 and Node.js (and it's technical steering committee) generally work well together and collaborate in good faith (at least in the last couple of years). There are some disagreements and different goals but the project has also helped standardize a bunch of things in the language (like Myles contributing a large chunk of top-level await).
Would it be OK to open a separate issue (or ping me at an existing one?) to discuss this and what node would have to do?
I don't think that enough was done and I consider this (as mentioned above) a mistake on the project. I know some work was done on helping maintainers as part of pkgjs but I don't think this particular area was considered. If it's OK with you I'll open a separate issue in the Node.js TSC repo and ping the technical steering committee?
Not that I am aware of. While I work for one of the member companies (Microsoft) my employer is actually not involved with my contributions to the project which I do on my spare time (I actually have a clause in my contract that allows this) and my work in Microsoft is on entirely different areas (not Node.js related at all). I can't read peoples' minds so I can't speak for other members (+I didn't attend the last summit unfortunately) but generally corporate interests are mostly in line of "We run clouds like Azure or GCP so it's good that everyone has the same Node.js and it's maintained in an open way and not by a single company". The OpenJS foundation started as the Node.js foundation and since Mozilla is not Node cloud-vendor I suspect they didn't join - but I certainly hope they do!
It is truly regretful in my opinion you feel you have to note this to have your voice heard.
I personally care about this and I feel many others in the project do too.
Your feedback has been very valuable and your perspective brings a lot to the table. I hope it's clear we're trying to listen and engage in good faith. |
Absolutely zero. What was discussed at the collaborator summit was driving a process for bundling and distributing certain open source ecosystem modules with the node binary. Modules that, as it turns out, are already distributed with the node binary thanks to other things that are bundled there. These would be either modules node itself uses or are considered core dependencies for most node.js apps. We discussed the possibility of how we would do that, clearly identifying that we would need to define a process for deciding what, when, and how. The issue I opened here #43413 starts to discuss one proposal for that process.
Let's be absolutely clear: there are no corporate overlords in Node.js, and the suggestion in this comment is insulting, quite frankly. The Foundation does not make the decisions here, and in fact, has absolutely no standing in this conversation at all. What happens in this project is driven by individual contributors doing what they think is right for the project. Reasonable people can disagree and we have processes for that. Just because one individual contributor opens a PR proposing something, it doesn't mean "the project" or the Foundation or nameless Big Tech overlords are doing something. It only means there is a proposal from one individual on the table for discussion. There is lots of reasonable discussion and debate to be had around whether and how to vendor bundle ecosystem modules in core. Lots of constructive feedback to have. But let's not throw around accusations of theft, and force, etc when it's literally just a conversation among individuals who disagree on how code should be distributed. |
I feel like this is a tangent point that was introduced while discussing a much more serious issue, so, sorry if I'm butting in, and doubly sorry if I've misunderstood, but please do not add color codes to log generation facilities. This works fine for development and toy environments but in professional or robust environments where logs are shipped to an aggregator, pumped through a content agnostic event stream, SIEM, etc, all this does is add work for the logging infrastructure and it comes across when we see it the same way that using a gmail account for a customer-facing client contact point does. The producing system should be making logs that are agnostic of how those logs will be rendered and focus purely on the content of those logs (vs. presentation), because they're not guaranteed to be the same system (which system produces logs vs. which system is rendering those same logs). By adding functionality that makes it easier to colorize log output, it would be creating incentive for developers who don't know better to build potentially (unlikely) beautiful bad practices. And yes, colorizing log output is a bad practice. This is coming from the system domain not software development domain (i.e. I am not a nodejs developer, I am a person that builds around the apps that would be created that use this functionality). If I have misunderstood what's being discussed please delete this, I recognize the seriousness of the broader discussion taking place. Just, again, please don't add color content to your logs so as to deter normalization of the pain it creates. |
@jasnell added. @Qix- I just want to add my 2 cents. The discussion on vendoring in some additional components has just started and there are a number of different considerations we need to factor in. Your input is one of those that we'll need to make sure we address along with others some of which we probably have not yet identified as well. I feel bad (as others have expressed as well) that they way its played out so far has caused you pain but its good that we are getting your feedback so it can be part of the discussion as it moves forward. |
To add one smaller point: one major use case for Node is to create powerful shell scripts, in a language more familiar than Bash (for example). Such scripts are often just a single file, and so there’s an incentive for script authors to try to achieve everything they want to do with Node’s builtins so as to avoid needing a |
This won't pass code review in most environments. It makes assumptions of the environment that it would have no way of knowing, doesn't account for differences in vision of the users, and shifts rendering responsibility to the component generating the text instead of the component rendering the text. It also introduces a very opinionated requirement (how color codes work, what colors they're mapped to) into the rendering environment. Colorized output is routinely rejected by most product teams. |
I’m intrigued by the idea of vendoring in packages into core, where a package like Chalk continues to live in its own repo and published to the npm registry, and maintained by its current maintainers and not us, and all that we do is include a copy of it within the Node binary and make it accessible via a specifier like With this approach, Node becomes just part of the distribution for Chalk, almost like we were including it as a dependency in our |
Not if the package remains ESM-only, though. |
Apologies for the huge delay. I've been dealing with COVID the last few weeks. Will try to cover as many unique points as possible.
Apologies for my hostile tone earlier, and I appreciate you reaching out to me elsewhere to clarify intent as well.
I also like color, but please believe me when I say this is very far from the general sentiment. Color in terminals is a great way to piss off a lot of people for a lot of very valid reasons.
The disk caching issue isn't node's to solve. An adequate package manager should cache same-version packages on disk if the user desires. That's neither Node's nor Chalk's problem.
If this is truly something Node wants to add to core, I see this as the only way forward paired with a few more specifics from other comments.
This is the worst solution in my opinion, which is in large part why the PR garnered such a visceral response from me to begin with.
I'm not well versed in Node.js committee operational specifics but I would be sad to see a platform such as Node.js be driven purely by independent business value. If that were the case, then ESM shouldn't exist because it's in all cases a non-zero amount of work to move from CJS/something else to ESM.
I think this is more a failure of a clear migration plan, hugely bloated tooling surrounding the change (package.json confusion, typescript confusion, versioning confusion, etc.) - paired also with, yes, failure to reach feature parity in a short amount of time (though I'm less convinced that's really a blocker).
I would like to see your data on this.
I disagree with your interpretation of download counts.
To me, it seems people don't upgrade at all on average. The module system has little to do with it, and in fact it seems the ESM version of Chalk has has much more acceleration than other versions if you look at things linearly. 1.4 million over 4 months is roughly 350k/mo. 10.8m / 25 months is roughly 432k/mo. And so on. Of course this isn't really a fair representation of these numbers but I can't seem to see how any assertions about module systems can be made based on chalk's downloads alone. Just want to mention, thank you for the offers for invitation. Happy to join whatever meetings you feel are appropriate - you can reach out on Twitter if you'd like, DMs are open. Otherwise E-Mail/GH pings/etc work too.
Which makes a lot of sense given the nature of bluebird. By contrast, Chalk is not introducing new language-level features or execution flow primitives like Promises. Further, a lot of what Chalk does is very subjective (explained in my follow-up comment).
Absolutely. It doesn't affect me personally too much (my sponsorships don't fluctuate much and it's very far from being substantial enough for me to rely on the income in any capacity). But this will be a concern if this becomes a standard operating procedure in the future.
I appreciate the insight.
I fully agree. Was the resolution of this issue a dependency for #43523? That isn't clear, if so.
Please consider this from the viewpoint of those of us who do not work at such companies and only have the Node committee to interface with paired with information about the OpenJS committee. It is not meant to be insulting. When there is an established pattern of this happening, it's only natural to be concerned.
This is how you perceived it because you had all of the context. However, in this issue as well as the PR, none of that context was conveyed to the public (i.e. the people who would be affected by this). That's all fine and well. It's the order of operations paired with the reinvention of the wheel and apparent deprecation-by-proxy of Chalk that was the more provocative aspect of this movement. @GeoffreyBooth: #43382 (comment) The same argument could be made about a lot of packages, thus I don't really agree it's a strong argument in its own right. @GeoffreyBooth: #43382 (comment)
Yes. I would imagine @sindresorhus would also be, though I cannot speak for him. And when does CJS get elevated to Stability:3? Or are we just fine with divergent ecosystems forever? What is your suggestion for managing packages that have to have both thus have problems with synthetic default exports, the endless addendums to When does the maintenance nightmare end, ladies and gentlemen? |
A few additional notes. Terminal styling is not straightforward. It is one of the most archaic-but-still-prevalent pieces of CS technology still around today. A lot of what is considered "standard" styling these days comes from a Wikipedia article that very loosely and only mostly-accurately reflects the various old as hell standards for terminals long since obsolete. Further, it completely ignores the only correct way of terminal styling, which is to consult the system's Terminfo database a la libcurses. Somewhere ages ago I discussed this at length and we decided against this functionality in Chalk (can't find it at the moment), but you can see some more thoughts about it here: chalk/chalk#370 (comment). I even wrote a parser for it in anticipation of this change. However, not doing this will shut out edge case terminal emulators as there is no agreed upon standard. There have been several attempts over the years to create one with all terminal emulator vendors but to my knowledge there has been no consensus. The benefit of Node already doesn't honor this on Windows - well, namely libuv's Is Node going to add the ability to ignore colors when the output isn't a TTY? What about when the output is a TTY but the underlying What about Truecolor? There's no way to detect it. But Chalk supports it. Will Node? Ultimately someone will have to maintain this. You already have a team of people who have a proven track record of doing so (comparing us to Marak is frankly insulting and I won't entertain the notion any further), and trying to stay up to date on the latest in the Javascript ecosystem. We can't make everyone happy with the packaging choices because the community is so intensely fragmented, so we've instead opted for ESM because it vastly simplifies things and allows for us to stay current. As we've indicated many, many times in many places, Chalk 4 and Chalk 5 are nearly functionally identical. Simply not upgrading is always an option. Also, not including a package because it uses Javascript features is a strange choice. If the answer is to re-invent the wheel, then I'm not really sure what else there is to say. It goes against pretty widely accepted best-practices. If anything, this has highlighted a much deeper issue with the ESM migration plan (which I'm beginning to suspect isn't even a thing, as CommonJS is still Stability: 2). Wasn't ESM supposed to be The Answer to most of these issues? This is why I inquired about the Node.js committee <=> TC39 relationship. Because it's been a nightmare for years. I'm really not sure what the answer is here. Everything proposed feels like it's a bandaid patch or an attempt to keep the status quo as it was 10+ years ago. |
@Qix- Personally I expect both module systems to be extant forever, yes. The problem here is that even if we could wave a magic wand and erase everything but ESM, it still wouldn't be better. Universal code is an impossibility - there are always going to be browser-specific use cases or server-specific use cases; dependencies will sometimes need to be loaded from disk and sometimes from a server, and configurably so; build-time optimizations will always be desired for smaller over-the-wire size or smaller memory footprint or to reduce i/o; etc. Of course, we can't wave such a magic wand either, so the only way the ecosystem can minimize maintenance burden - for everyone, including maintainers of packages and platforms as well as open source and enterprise consumers - is to maximize interoperability. A CJS-only package achieves that modulo a bundling process for browsers that everyone already has in place; a dual package achieves that at the cost of an additional build process for the package prepublish, but still requires a build process for non-node environments anyways, thus adding no value; an ESM-only package forces someone who didn't write the code to change its semantics in an unsafe way in order to use it in node. Whether ESM was supposed to be The Answer or not is likely a long and interesting discussion, but it's also irrelevant - it simply isn't, and won't be. |
So what was the point of ESM, then? 🤔 |
Okay, I’m persuaded that this isn’t something that the Node core team wants added to our maintenance burden 😄 especially if there’s some way to get the benefits without the costs, in that we could distribute Chalk without needing to also maintain it (#43413). Though there are issues with that as well.
CommonJS won’t become legacy/deprecated anytime soon, if ever; see #33954. That said, though, the only development happening with it is as a result of continued work on ESM (stuff like
There is no “ESM migration plan.” There’s the loaders roadmap, which is work on ESM loaders to add functionality like observability and mocking to ESM for parity with CommonJS. We haven’t been focusing on what to advise users after this work is done, as it’s irrelevant for now; and the ecosystem already does a great job in choosing the patterns and technologies it wants to elevate without us adding our opinions to the mix. If pushing forward ESM is of interest to you, we could certainly use the help on the loaders team. |
It would be beneficial if the ESM migration plan was discussed in a separate issue and we bring back the original topic: adding an explicit string formatting module to core. @benjamingr could you open a separate issue to discuss the impact of core decisions on individual/module sponsorships? |
This comment was marked as outdated.
This comment was marked as outdated.
This definitely is still an important use case to meet in core, I think, despite the complexity. |
This comment was marked as outdated.
This comment was marked as outdated.
bump |
Fixed by #51850 |
What is the problem this feature will solve?
Node.js has multiple formatting related APIs such as
util.inspect()
,util.format()
,util.formatWithOptions()
,util.stripVTControlCharacters(str)
and multipleutil.inspect
sub APIs such ascolors
,custom
,styles
anddefaultOptions
.We recently added
util.parseArgs()
to the util module and it becomes pretty big overall, while not all really being connected with each other.What is the feature you are proposing to solve the problem?
It is also considered to add new APIs such as a nicer abstraction for colors. To do so, I would like to create a new module that only contains string formatting APIs including the above. These APIs should always be kept supported from the util module as reference to the new module.
@nodejs/util @nodejs/tooling @nodejs/tsc any opinions?
The text was updated successfully, but these errors were encountered: