-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[Question] How can we better support TypeScript? #1261
Comments
This works:
But that imports all of validator. In version 11 these types of imports used to work:
I assumed that this was tree shakeable. I'm using validator in this project: https://www.npmjs.com/package/@fireflysemantics/is And each function is a typescript export. If validator is imported as a whole, then all the exported functions will include all of validator. It would be great if they would only include the specific function, but that seems to be enabled for ES6 only ... not typescript ....? |
I think There's another requests to add our own typing here, perhaps as the primary source of truth; and then mirror it on _DefinitelyTyped`, see conversation in #1194 #789 /cc. @johannesschobel |
If you offer own typings, you don't need to add them to DT anymore. |
Hmmm ... I upgraded this library to the latest version of validator using the import I described: https://www.npmjs.com/package/@fireflysemantics/is But now I'm getting errors:
So I'm thinking the easiest thing to do is just rewrite the library in typescript and offer a Thoughts? |
@fireflysemantics -- I wouldn't call that the easiest option though... |
I think I'm going to have a go rewriting the whole thing in Angular Package Format. https://docs.google.com/document/d/1CZC2rcpxffTDfRDs6p1cfbmKNLA6x5O-NtkJglDaBVs/mobilebasic This will compile support in for all the popular package formats and provide typescript typing. Did a small write up here: https://medium.com/@ole.ersoy/creating-an-angular-9-npm-library-1a658ecfa3dc When TS types are supported separately then Typescript is always playing catch up and as a result upstream builds break. |
Here's what the
This would be exported from the public surface (
And import goes to:
from:
IIUC this should produce tree shakeable utility methods. |
@profnandaa , i wouldn't say, that it is the "go to strategy" to use DT. The major issue with DT is, that it is a separate repostiory where you need to publish your changes. Often, the library repository is managed by the core-developer team, and the DT repository is maintained by the community. So the "DT interface" often differs / does not reflect the current state of the library itself. I mean - the cleanest (!) solution would be to re-write the entire library in TS and automatically build the JS version from there (i.e., use a GitHub action to build the JS version after every push to |
So, a rewrite is out of question. Looks like viable proposals we have right now are:
Personally I'm leaning towards 3. I'm also trying to:
Let me know what you folks think. |
From the practical point of view, I think the only viable solution is 1 |
I'm almost done with the rewrite. I'll push it to this repository when it's complete: https://github.com/fireflysemantics/validatorts I'm hoping you will accept the changes and fork it back. This is what I have done: Angular Package FormatThis is what I used. It means that all the popular package formats will be supported right out of the box (UMD, ES6, commonsjs, etc.) with each compile. The Angular CLI takes care of it. It also means we get a simple flat tree shakeable API surface for all the validators and sanitizers that is consistent across all module formats, and that best practices are followed by default. This improves the usability and simplicity of ValidatorJS. TypeDocAdded Typedoc documentation so that the the API is easily publishable in the Github Pages of the build Named Exports OnlyThis means better tree shaking and refactoring for clients of the library. See this article: https://blog.neufund.org/why-we-have-banned-default-exports-and-you-should-do-the-same-d51fdc2cf2ad Jest for TestingEach validator will have a Pure Typescript ImplementationSo that the types are generated for free and the library is always up to date. There is 0 chance of errors or other issues like the ones I'm now dealing with popping up, because the Typescript implementation is the yin and yang. Separate Module Folders for Sanitizers and ValidatorsSo that it's easier to see what is what. So we have:
Consistent Parameter Names
Nice to HaveI think the outdated Typescript Inline Typedoc examples In @fireflysemantics/slice I inlined API examples like this: https://fireflysemantics.github.io/slice/doc/classes/_src_estore_.estore.html This would be nice to have for validator also. I won't do this in the initial release, but I think it would be helpful for the community as a whole. The Example:
Explicit Integer conversionsSome parts of the code reference arrays like this:
Converting these explicitly like this:
parseInt(parts[1])
Conversion Notes: |
Another thing I noticed is that Typescript "Caught" some things that I think are errors. I've asked about them in other implementation related issues for |
@fireflysemantics -- please check previous conversation above and respond. |
@profnandaa my response is outlined above, I'll do the initial push hopefully today or tomorrow, and ya'll can see what you think. At a minimum Typescript users will have a consistent source to go to, while also gaining better tree shaking, the same consistent import set, and better refactoring since names will always be consistent. |
I pushed the files: https://github.com/fireflysemantics/validatorts This is the Angular Library Package Format Section of the project: https://github.com/fireflysemantics/validatorts/tree/master/projects/validatorjs/src I'll deploy to verdaccio now and test out imports. When all is well I'll also add it to NPM. Clone and build with the Angular CLI using |
This is what the
Notice the module formats that are now generated for free, in addition to typescript definitions. Here's a stackblitz demo: https://stackblitz.com/edit/typescript-validatorts-demo?file=index.ts Here's the NPM version: ValidatorJS now has:
And once the tests are translated, easier access to per validator unit tests with Jest. |
Why do you use angular and not "pure" ts? Angular will add a lot of dependencies for this project that may not be needed at all! |
@johannesschobel The build is using the Angular Package Format. Look at the The benefit of using the Angular Package Format and the Angular CLI for the build is that we get all the module formats built for us for free, in addition to the type definitions. The rest is the same. If we use pure TS, we will not get all the package formats (UMD, etc.) built automatically. |
Hello, |
@tux-tn The rewrite is done and there are lots of other benefits, including now having the same paths for all imports regardless of whether you are doing tree shaking or not. For example this has three issues:
Default ImportsOne is it used default imports so developers don't have to call the Longer Import PathThe second is that when you want to have a more efficient application with tree shaking you have to use a longer import path. Communication and Project ManagementSo some developers on your team might use that and some might not. And now you added an additional communication requirement around that, in addition to having to document the different paths and different formats. Lag in Typescript SupportAnd finally if you keep the Typescript paths separate then there's always lag with respect to version updates in ValidatorJS or the Typescript types, which is why I did the conversion. More Core Project Development EffortSince Typescript is separate so is the task of keeping up with what has changed. It's additional effort for no real reason. There's hardly any difference between the Typescript implementation I did and the original current JS implementation. BUT there are a lot more benefits to the Typescript implementation. Broken Typescript SupportExporting single Second when this project starting using this anti pattern it broke the current Typescript definition support and it's very easy, given the current circumstances, for this project to do so again in the future, since the Typescript definition types are not generated via the implementation. |
it's not a default import, you can have
This was never a problem in the javascript ecosystem. If you think about it as a matter of code size, the difference is ridiculous and most people are using module bundlers.
This is why i am saying your rewrite doesn't make sense and it will be better for the community to participate in the current conversation rather than try to make your own fork. |
It's fine. I respect that ya'll want to stick with what you have. I needed the conversion and will be adding more tests / typed interfaces for options / etc. and this will make the development experience much richer for me personally. Who knows maybe ya'll will come to like it as well one day :) |
@fireflysemantics the beauty of open source is that you are free to do or use whatever you want. Good luck with your project 😃 |
Added these types for
I believe the last three are missing regexes in the corresponding |
@fireflysemantics -- looks like you met your objectives, I'll go ahead a close this issue, and open a fresh one to discuss viable TS options we would like to take for the library. Thanks! |
From an end-user's perspective, I don't see why "a TS rewrite is out of the question". I realize that from the developers' perspective that might well be the case, but as a user, I just want a clean TypeScript source that I can import without all the shenanigans like #1208, #1204, #1015 etc. It's 2020 after all. |
It seems TS not implemented yet and still we need correctly type definition for better using of validator in ts projects |
Trying to use
@types/validator
and getting errors. I asked the question on SO here. Curious if anyone else is having the same issue?https://stackoverflow.com/questions/60457757/typescript-imports-for-valiatorjs-failing
The text was updated successfully, but these errors were encountered: