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
Minor version 2.2 is breaking for TypeScript consumers #215
Comments
Sorry about the trouble, but this seems more like a problem with the DefinitelyTyped way of handling typings. Projects can't really take into account unofficial type definitions. We went with a default export now as that's more future proof. It's not a breaking change, as Chalk didn't officially come with typings before. Maybe you should open an issue on TypeScript and request that |
Shouldn't the TypeScript typings reflect what's available at runtime / to JS consumers? |
@felixfbecker It's going to be years until we can use native modules and default export in vanilla JS. I don't see why TS users should have to wait. |
I don't think so unfortunately - but what is better about import chalk from 'chalk' vs import * as chalk from 'chalk'
import { red, green } from 'chalk' ? |
Chalk is by nature a default export. The fact that you can do You will have to change |
Makes sense. Feel free to close or leave this issue open a few days for TS folks who might meet the same issue and come here |
Thanks for understanding. I'll keep this issue open for a while. |
I've opened a TypeScript issue: microsoft/TypeScript#19283 |
Another issue I have with the new TypeScript definitions: function logBold(msg): string {
return chalk.bold(msg)
} error TS2322: Type 'Chalk' is not assignable to type 'string' import * as chalk from 'chalk' being broken is fixed for me with a search+replace to import chalk from 'chalk' |
@timsuchanek You're passing an implicit any (msg) to chalk, which only accepts a string. If you add the correct type your error will go away. |
I also believe current definitions are not done okay and should be exported as |
I've also just been broken by this. Despite initial thrashing, the changeover is not too painful. Since examples are useful I thought I'd share the changes I made to switch to this: Previous import examples: import { constructor as ChalkConstructor, Chalk, ChalkChain } from 'chalk'; Now looks like: import chalk from 'chalk'; Usage looks like goes from this: const colors = new ChalkConstructor({ enabled: loaderOptions.colors });
const makeLogInfo = (loaderOptions: LoaderOptions, logger: InternalLoggerFunc, green: ChalkChain) =>
//...
function successfulTypeScriptInstance(
// ...
colors: Chalk,
// ...
) {
// ...
} To this: const colors = new chalk.constructor({ enabled: loaderOptions.colors });
const makeLogInfo = (loaderOptions: LoaderOptions, logger: InternalLoggerFunc, green: typeof chalk) =>
//...
function successfulTypeScriptInstance(
// ...
colors: typeof chalk,
// ...
) {
// ...
} PS every time a package on npm starts shipping it's own |
Created a PR to remove typings from Definitely Type |
Sorry, 5 days passed, but chalk 2.2 is still not usable for TypeScript 2.6. Could you please release a new version to make chalk 2.2 compilable again? As workaround I have set version to 2.1, but :( it is not good. |
I'm not sure that's true. ts-loader runs against TypeScript@next and it's okay: https://travis-ci.org/TypeStrong/ts-loader However, 2.2 did seem to be a breaking changes release |
Thanks for the link - it seems that ts-loader is okay with TypeScript@next so my working assumption is that this is OK. Can you think why that wouldn't be the case? I'm sure I'll learn quickly if it isn't! |
@sindresorhus Totally understand the issue with the unofficial type definitions, and absolutely agree with the XKCD. Regardless, IMO, the type definitions output with On the first, I would expect that to behave just like Like I said, maybe it's just me, but I found it unintuitive since I never import like |
@develar TypeScript 2.5 is the latest official version. 2.6 is still RC from what I can tell. |
You can't compare it to
The
The Chalk colors are part of the Chalk class and not part of the exported interface directly for reasons explained in my earlier comment.
How would you expose type-strong options? We're always happy to consider improvements, but we're not interested in changing how the default export works. That's set in stone. |
A new release is out with some TypeScript improvements: https://github.com/chalk/chalk/releases/tag/v2.3.0 |
@sindresorhus is that test file only compiled or actually executed? Judging from the package.json it looks like the file is only compiled, which is expected to work, but it will be a runtime error Line 11 in 14e0aa9
|
@felixfbecker That's a good point. We should execute it too. |
Is there any way to have named constant here or at least some kind of documentation for those magic numbers? |
Sure, you can explain it in a docblock of the type alias. Will be more visible though if explained on the actual parameter/property that uses it. Or, provide it at runtime for JS consumers too ;) |
Maybe this is a stupid question but why don't we expose typescript bindings in the respective module, |
Didn't know that. I loved my Somehow the "downsides" never really applied to me:
The generic names made my easy to read, I never really had troubles with name conflicts and I never needed more than three or four colours at once. (Also auto-importing and removing unused imports made this very ergonomic.) |
@Qix- Too much overhead and boilerplate. I don't want TS boilerplate in all of our tiny modules. Almost nobody uses |
That won't happen. As I've commented earlier, you can just do: import chalk from 'chalk';
const {red} = chalk; |
Yeah, I read you comment and I know I can do that. Just wanted to give some feedback, because I personally prefer the other way and haven't encountered downsides. |
@donaldpipowitch Position is that |
The const enum is documenting it -- It also compiles to the actual values so the above snippet becomes var chalk_1 = require("chalk");
chalk_1.default.level = 2 /* Ansi256 */;
|
@sindresorhus thanks for Chalk and your other contributions! You may not appreciate it, and I know you didn't author it, but Also, I've used upwards of 100 typed packages over the past year, and this typing is the first to require a non-namespaced For other people wondering why chalk broke for them with a minor release, as stated above, change import * as chalk from "chalk" to import chalk from "chalk" |
@calebboyd even with |
@felixfbecker Yep! That only effects what typescript emits (your code). Not declarations within types provided by modules you're using (eg chalk). @mceachen import * as chalk from 'chalk'
console.log(typeof chalk) //function The above syntax for extracting an object of type The previous types also did not capture |
- Update dependencies and devDependencies to latest versions - Fix chalk import (see chalk/chalk#215) - Fix git merge command
@celsoe that's not related to TypeScript - that's because you're running it in a wrapper of some sort that is causing |
modify {
"compilerOptions": {
"module": "commonjs",
"target": "es6",
"noImplicitAny": false,
"moduleResolution": "node",
"sourceMap": true,
"outDir": "dist",
"baseUrl": ".",
"typeRoots": [
"node_modules/@types",
"node_modules/types" //it's for chalk
]
},
"include": [
"src/**/*"
]
} |
sourcegraph/javascript-typescript-langserver#371
https://travis-ci.org/sourcegraph/javascript-typescript-langserver/jobs/289329597#L1487-L1490
Before 2.2, with
@types/chalk
, it was possible to import chalk like this:https://github.com/sourcegraph/javascript-typescript-langserver/blob/6fcc4e1df9283dbd84db9411aa26e9f49556cb50/src/logging.ts#L2
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/05ac46f9a9307d43e082b7cec4b32f40df3a3b58/types/chalk/index.d.ts#L6
2.2 brings its own types, which override
@types/chalk
, but they don't allow this import (only a defaut import):chalk/types/index.d.ts
Line 90 in d86db88
Although it seems to work at runtime and appears semantically correct / correct in regard to the JS:
chalk/index.js
Line 219 in d86db88
The text was updated successfully, but these errors were encountered: