Skip to content

Conversation

@vzhidal
Copy link
Contributor

@vzhidal vzhidal commented Jun 3, 2016

Hi @loedeman , I've added UMD support today. It's easier to debug in node and integrate with modules systems now. It should work like previously if library is used without any modules system. Please, have a look. Thank you.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 94.093% when pulling 403160b on vzhidal:master into 55aece9 on loedeman:master.

@vzhidal
Copy link
Contributor Author

vzhidal commented Jun 3, 2016

p.s.
should resolve issues with webpack integration (and any modules builders)

@loedeman
Copy link
Owner

loedeman commented Jun 8, 2016

Wow, sounds great! Thanks! I will merge your pull request as soon as possible ;) !

@loedeman loedeman merged commit 403160b into loedeman:master Jun 8, 2016
@loedeman
Copy link
Owner

loedeman commented Jun 8, 2016

Hi @vzhidal,

Since you are already rocking by sending two pull requests 💯, I am wondering if you could perhaps help me by writing a (couple of) Jasmine unit test(s) to 'prove' UMD/WebPack functioning correctly with AutoMapper-TS? I don't use AutoMapper with UMD myself, but would certainly like to have all situations in use covered by tests. It would cost me some extra time to dive into UMD/WebPack myself, which I currently lack a bit...

Cheers, Bert

@vzhidal
Copy link
Contributor Author

vzhidal commented Jun 8, 2016

Hi @loedeman,

I've just added an additional step in gulp. Your library is wrapped by the gulp-umd plugin that has it's own tests. Do you think it is needed to cover a bundled version of your library? If it is so, could you provide your ideas how specs should look like and where they should be placed? (As I understand all current specs are written on TS).

@loedeman
Copy link
Owner

loedeman commented Jun 9, 2016

Hi @vzhidal,

you are right, currently tests are written in TypeScript. However, those tests are compiled and run against the compiled source as well ;) . In the end, it's not the TypeScript code that matters, but the resulting JavaScript code. That's why I would love to prove certain scenarios working (traditional, UMD, WebPack).

Maybe, given the fact gulp-umd is tested already, it's juist samples in wiki I would like ;) ... Should it be unit tests (each one in its own test file), the only thing they should test is their way of calling AutoMapper, for instance just proving ability to access the AutoMapper object.

Cheers, Bert

@vzhidal
Copy link
Contributor Author

vzhidal commented Jun 9, 2016

Hi @loedeman, sorry, but I do not have enough time for investigating the tests infrastructure. So I have a few questions for you. To test AMD and CommonJS modules systems a few objects in a testing runtime should be mocked . In case of AMD - define function (with amd property) should be available in a global scope. In case of CommonJS - exports and module.exports should be defined. In other case UMD will define a namespace for a library in a global scope. So mocks should be added to a global scope before loading a bundled library. Global scope should be cleared and bundled library added for every module system separately. Do you know how to configure karma for doing this?

@loedeman
Copy link
Owner

loedeman commented Jun 9, 2016

No problem if you don't have the time to investigate the test infrastructure. I will look into it sometime in the future (maybe ;) )... It's no deal breaker to lack those tests, since the code is already working and tested (!) by the gulp-umd module, as you correctly stated yesterday.

I think a separate Karma process should be created for each module system. I did use AMD in a project a couple of years ago, and had to overhaul Karma configuration big time while moving to AMD ;) ... No problem, but a lot of effort for little gain ;) ...

@vzhidal
Copy link
Contributor Author

vzhidal commented Jun 9, 2016

agree :)

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.

3 participants