search

hmil / tslint-override

TSLint plugin bringing support for the override keyword
MIT License
27 stars 6 forks source link

Offer option for more Angular like Syntax #29

Closed wSedlacek closed 4 years ago

wSedlacek commented 4 years ago

Fixes #28

hmil commented 4 years ago

I added a test and changed a few things so the test would pass.

wSedlacek commented 4 years ago

Using globalThis will require upgrading to TypeScript 3.4 or higher. Is there a reason we are still using TypeScript 2.9.2? Just for compatibility or something else?

hmil commented 4 years ago

It's for compatibility.

Granted pre-3.0 stuff is largely outdated, but 3.4 is a fairly high requirement at the moment (people stuck on old angular versions for instance may have to invest a lot of effort to migrate to newer versions and get access to TS3.4). If there's a way to be compatible with a wider range of versions I'd prefer that.

Are you sure the solution used in the current register.ts doesn't work when node.js typings are absent?

wSedlacek commented 4 years ago

You do get an error if "node" is not included in the typings. Screen Shot 2020-02-13 at 22 41 22

Is there another check we could run other then typeof global === 'undefined'?

wSedlacek commented 4 years ago

Would this work?

export const ctx = this;
ctx.Override = () => () => {
  // noop
};
wSedlacek commented 4 years ago

Finally got it to pass with types not including node by setting global with this line

declare var global: NodeJS.Global;

I think I am finally happy with this and it should be backwards compatible. Let me know if you want me to change anything else.

wSedlacek commented 4 years ago

Cleaned up commit history

wSedlacek commented 4 years ago

Making a library is a lot harder then working on your own project 😅 Thanks for bearing with me!