search

vitalets / angular-xeditable

Edit in place for AngularJS
http://vitalets.github.io/angular-xeditable
MIT License
1.91k stars 404 forks source link

Change main entry to index.js and export module name #753

Closed madflow closed 5 years ago

madflow commented 5 years ago

With this PR it is possible to to do:

import angularXeditable from 'angular-xeditable';

angular.module('app', [angularXeditable]);

and have on less verbose string to worry about in Angular 1 ;)

ckosloski commented 5 years ago

do all the tests still pass?

madflow commented 5 years ago

I followed https://github.com/vitalets/angular-xeditable/blob/master/CONTRIBUTING.md with node 10 and node 4 on the master branch.

npm run build produces an error (something about jsdoc) - npm run start works. Some docs tests pass - none of the dev tests do.

So - since the master do not work with my setup - I did not bother to test with this PR.

I cannot see any references to the main entry anywhere in the code though...

ckosloski commented 5 years ago

Might be the version of npm you have. When you run the site (http://localhost:8000), does everything still work?

madflow commented 5 years ago

Well - I ran yarn run start and clicked around a little bit. Looked O.K.

ckosloski commented 5 years ago

I ran the tests with these changes and they all passed.

eugef commented 5 years ago

@madflow thanks for your PR.

Could you also update the Readme here https://github.com/vitalets/angular-xeditable#insert-dependency

I suggest we also switch to this way of importing in our docs https://github.com/vitalets/angular-xeditable/blob/master/docs/js/app.js to prove it works

madflow commented 5 years ago

@eugef The use case for this is only when some kind of asset/module bundling is in place (Webpack, Rollup, Parcel). "Importing" angular-xeditable has always worked - but now it simply exports the module name, too. This is, because Webpack uses the main entry in the package.json file. I can document this in the Readme - no problem. Switching the docs is a story that would exceed my intentions with this PR.

eugef commented 5 years ago

@madflow ok, let's not touch the docs. Thanks for updating the readme.