gregjacobs / Autolinker.js

Utility to Automatically Link URLs, Email Addresses, Phone Numbers, Twitter handles, and Hashtags in a given block of text/HTML
MIT License
1.48k stars 238 forks source link

Fix commonjs types (previously the issue: 'Please correct @types') #249

Closed shyamal890 closed 5 years ago

shyamal890 commented 5 years ago

It seems the replaceFn types are wrong.

gregjacobs commented 5 years ago

Hey @shyamal890. I actually don't own the @types library for Autolinker (but am happy that someone put them together!). I think you need to raise an issue with DefinitelyTyped.

I'm working on v2.0 though which is a conversion to TypeScript and will have the correct declaration files emitted within the package itself

gregjacobs commented 5 years ago

254

michahell commented 5 years ago

Hijacking this thread for a question regarding the AutolinkerConfig, how to import this type ?

the following works: import Autolinker from 'autolinker';

but the following does not: import {Autolinker, AutolinkerConfig} from 'autolinker';

even though I can see in the typings that the interface is exported... I don't understand :/

gregjacobs commented 5 years ago

Try:

import Autolinker, { AutolinkerConfig } from 'autolinker';
michahell commented 5 years ago

unfortunately for me that doesn't work. forgot to mention that I'm using "autolinker": "3.0.3", "@types/autolinker": "0.24.28", "typescript": "3.2.4"

I solved it as follows, for now, because only the linker option hashtag caused errors:

Type 'boolean' is not assignable to type 'false | "twitter" | "facebook" | "instagram"'

(because of implicit false => boolean type mapping if you used hashtag: false):

interface AutoLinkerConfig {
  [index: string] : any;
  // tslint:disable-next-line:max-union-size
  hashtag: false | 'twitter' | 'facebook' | 'instagram';
}
michahell commented 5 years ago

OK the above solution only worked for default Angular compilation, but not for our unit tests for some reason.

I tried both using AutoLinker statically, as follows:

body = Autolinker.link(body, this.autoLinkerOptions);

and as an instantiated class method:

this.autoLinker = new Autolinker(this.autoLinkerOptions); body = this.autoLinker.link(body);

I'm commenting out our use of Autolinker for now as I've no time to debug the following Jasmine issue:

TypeError: autolinker_1.default is not a constructor

gregjacobs commented 5 years ago

Hey @michahell. Thanks for posting your autolinker and @types/autolinker versions.

Your main issue is probably that @types/autolinker is for Autolinker v0.24 rather than v3.0.3. I mentioned earlier in this thread that you no longer need the @types package because Autolinker actually ships it's own types now, so the @types package is likely conflicting with the v3.0.3 package.

Try uninstalling @types/autolinker and give it a shot. Let me know if you still have errors, we can probably work through them.

michahell commented 5 years ago

Hi Greg, yes this is one of the first things I tried (ran into @types being obsolete in other packages as well) but that does not change the unit test from failing to run :(

I think the problem is somehow in the differences between our tsconfig.app.json and tsconfig.spec.json, where in the former we use:

"compilerOptions": {
     //...
    "module": "es2015"
  },

and in the latter:

"compilerOptions": {
     //...
    "module": "commonjs",
    "types": [
      "jasmine",
      "node",
      "office-js"
    ]

I also tried adding autolinker to thetypes` property but that doesn't seem to do anything. I don't know to tell Angular / TS to include the types for autolinker and I think that's where the problem is currently.

gregjacobs commented 5 years ago

Btw, make sure to type your autoLinkerOptions property as AutolinkerConfig. If you have it as an anonymous object, you might get that error that a boolean (which can have both true and false values) is not assignable to false | 'twitter' | 'facebook' | 'instagram'

michahell commented 5 years ago

yes, but then I run into the problem that I can't import AutolinkerConfig :(

The following: import Autolinker, { AutolinkerConfig } from 'autolinker'; does not work for me, for some reason:

ERROR in src/app/components/render-message-body/render-message-body.component.ts(9,22): error TS2305: Module '"../../../../node_modules/autolinker/dist/commonjs"' has no exported member 'AutolinkerConfig'.
gregjacobs commented 5 years ago

Oh noes :( Ok, that should definitely work. I'll check this out when I get home and see what's up.

In the meantime, you could type your configuration as any

gregjacobs commented 5 years ago

Hey @michahell, apologies, missed this comment before:

I also tried adding autolinker to the types` property but that doesn't seem to do anything. I don't know to tell Angular / TS to include the types for autolinker and I think that's where the problem is currently.

It's actually not a problem with telling TypeScript about the types in your tsconfig.json. That's only needed if you use globals like Jasmine's describe and it functions which don't have matching import statements.

It seems there might be a problem with Autolinker's commonjs build itself. Are you able to switch your module system to es2015 on your tests? I'll try generating a new angular app tonight and see what's up with the tests

gregjacobs commented 5 years ago

@michahell Found the issue. I wasn't exporting the AutolinkerConfig interface at all 😭 I guess most uses of Autolinker are with the inline config object such as:

Autolinker.link( 'Hi google.com', { newWindow: false } );

But I'm glad you came across a case that you kept the config under a separate identifier!

This is now fixed in v3.0.4. Give it a try and let me know how it goes!

Best, Greg