Skip to content
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

Add gulp-util.json #832

Closed
wants to merge 3 commits into from
Closed

Add gulp-util.json #832

wants to merge 3 commits into from

Conversation

demurgos
Copy link
Contributor

Typings URL: https://github.com/demurgos/typed-gulp-util

Questions (for new typings):

  • Does the README explain the purpose of the typings and have a link to the JavaScript project?
  • Do the typings follow the source structure (e.g. index.js <-> index.d.ts)?
  • Are they external or global modules according the source (e.g. see README sources)?

Change Summary (for existing typings):

@demurgos
Copy link
Contributor Author

I just read about the namespace hack. I am using it internally: should I update my definitions ?

@demurgos
Copy link
Contributor Author

Well, I checked some other typings and it looks like they are also using import * as ... internally, so I'd like to know if my definitions are valid or not.

@felixfbecker
Copy link
Contributor

No, they are semantically incorrect by ES6 spec. It's a hack. For modules that use export =, use import x = require('')

@demurgos
Copy link
Contributor Author

Thank you!
It's been 3 months since I coded in TypeScript and I forgot some of the details about how to handle this. (I completely forgot the import foo = require(...) syntax. I'll update it.

@demurgos
Copy link
Contributor Author

I updated my definitions to use the latest version of vinyl and fix the imports. I could test it with some other projects and it seems to work fine.

@felixfbecker
Copy link
Contributor

LGTM. One minor note: https://github.com/demurgos/typed-gulp-util/blob/master/index.d.ts#L17
What is the reason to export this as a type? You can easily use typeof colors to get the type, but you are breaking the naming convention of uppercase names for types here. Also applies a few lines further down again.

@felixfbecker felixfbecker mentioned this pull request Oct 18, 2016
24 tasks
@demurgos
Copy link
Contributor Author

demurgos commented Oct 18, 2016

This is a remaining from when I was trying to re-export namespaces. Since there is no export namespace File = Vinyl syntax, this was the closest I could get so I just tried it - but since it does not work I used a normal namespace for File and forgot to remove it for the others.

I just fixed it at the moment.

@felixfbecker
Copy link
Contributor

Oh btw you can also just do export {colors, date}

@demurgos
Copy link
Contributor Author

Closing as gulp-util is deprecated

@demurgos demurgos closed this Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants