alexrainman / nativescript-carousel-view

CarouselView plugin for NativeScript
MIT License
16 stars 5 forks source link

NPM package tweaks #2

Closed bradmartin closed 7 years ago

bradmartin commented 7 years ago

To reduce the footprint (albeit small) ignore the .ts files excluding the typings and the .map files since they aren't needed for consuming the plugin.

So in the .npmignore I usually do:

*.ts
*.map
!carousel-view.d.ts
alexrainman commented 7 years ago

Yes, i was wondering how to do that :) thanks

bradmartin commented 7 years ago

While on this topic another thing that I believe TypeScript recommends is when the .d.ts file isn't index.d.ts you should have a typings key in the package.json

typings: carousel-view.d.ts

I've also found this helps certain IDEs resolve packages really well, some are smart enough I suppose to figure it out but specifically VS Code this seems to help a lot.

NathanWalker commented 7 years ago

@alexrainman it may be beneficial to download this seed: https://github.com/NathanWalker/nativescript-plugin-seed Then just drop all your code in that setup. Would be easy way forward to get a clean repo with npmignore setup outta box to get going for publishing.

alexrainman commented 7 years ago

@bradmartin @nathanwalker let me clean up the house :)

alexrainman commented 7 years ago

@NathanWalker first time i started to work on the plugin i tried your seed but couldn't make it work, so that's why i did it in my own way :)

bradmartin commented 7 years ago

Repo looks good, just add the demo app back in and perfecto 👍

alexrainman commented 7 years ago

Working on the demo :)

bradmartin commented 7 years ago

Might want to remove the Requirements from the beginning of the readme, that's not necessary for consumers of the plugin (shouldn't be at least). Maybe add a 'Contributing' section with that for async/await support.

alexrainman commented 7 years ago

Actually, the plugin needs async-await :)

bradmartin commented 7 years ago

Yea but not consumers of it. The shipped .js on npm shouldn't need to be transpiled or anything since you only need to ship the .js files.

alexrainman commented 7 years ago

You are right. I am doing some testing before moving forward. I installed from npm in a project with --tsc and module cannot be found. Maybe because typing was missing?

bradmartin commented 7 years ago

Possibly, but also IDEs resolve them differently. Best solution I've found is to remove the declare module... from the typings file and add the typings key to the package.json as I mentioned before. Make sense?

So don't need this and it should resolve the package much better: https://github.com/alexrainman/nativescript-carousel-view/blob/master/carousel-view.d.ts#L2 don't forget to remove the closing }

bradmartin commented 7 years ago

Oh and if you're using VS Code by chance, it's pretty awful at resolving added modules. I typically have to restart for it to find the module :/ I think that's an issue on VS Code repo, never had time to dig in to that.

alexrainman commented 7 years ago

Typings solved the problem. Adding the demo :)

alexrainman commented 7 years ago

Actually, i need to keep the requirements because it doesn't works without the helpers.js

alexrainman commented 7 years ago

Or, should i distribute that file with the plugin also?

bradmartin commented 7 years ago

I'd distribute, not everyone will use TS so no need to limit adoption :)

alexrainman commented 7 years ago

OK

alexrainman commented 7 years ago

How you do async/await when you use JS instead if you don't have TypeScript 2.1.0-dev requirement?

bradmartin commented 7 years ago

So you coded with TS, and transpiled to the .js so the transpiled .js will run since TS isn't actually executed. In your tsconfig.json, what are you targeting? ES5, ES6?

alexrainman commented 7 years ago

Yes, i understand that part now, but some of the custom methods the plugin offers needs async/await. How you do that with pure JS if you don't add the requirement?

await carouselView.insertPage(carouselView.itemsSource.length, person);
carouselView.setCurrentPage(carouselView.itemsSource.length - 1);
alexrainman commented 7 years ago

I am seeing the generated JS in main-view.js and it it will be quite complicated to do that from scratch :)

bradmartin commented 7 years ago

Ok, think I'm following you. You mean how do consumers of the plugin use await?

alexrainman commented 7 years ago

Yes if they use pure JS and not TS.

bradmartin commented 7 years ago

I don't think there should be a need for a JS consumer to use async/await. They'll have to go the tough route, but even then I'm not entirely sure. This can get confusing without seeing the code and trying :)

bradmartin commented 7 years ago

Because plain JS doesn't have await so they can't do that

bradmartin commented 7 years ago

They could use a Promise

alexrainman commented 7 years ago

Well, we should provide examples for that because "insertPage" and "removePage" should use await :)

bradmartin commented 7 years ago

I can get you a sample of that once you get it ready. I'll pull the demo and work with it some.

alexrainman commented 7 years ago

The demo will use TypeScript, i mean some code snippet to put on the README.

bradmartin commented 7 years ago

Yep sounds good 👍 I'll have to tinker with the code to nail the correct way without TS. Normal promise usage though:

function addSomething() {
   return new Promise( function(resolve, reject) {
          try {
                // do some work and resolve anything or nothing
                resolve(); 
                // or if the work here fails reject it
                reject('This failed because whatever'); /// or pass true/false whatever really.
           } catch (err) {
               reject(err)
           }
   });
}
alexrainman commented 7 years ago

How i can ignore .vscode folder?

bradmartin commented 7 years ago

in the ignore just .vscode/ don't think you need the / really but that should work either way

alexrainman commented 7 years ago

Done!

alexrainman commented 7 years ago

Where's that article? :)

alexrainman commented 7 years ago

@NathanWalker @bradmartin 1K downloads and counting, in just one day :)