nabil-boag / angular-multimocks

Tools for managing mock data scenarios in AngularJS applications.
MIT License
35 stars 11 forks source link

Deduplicate scenario data #59

Closed hbk619 closed 7 years ago

hbk619 commented 7 years ago

Previously a scenario had all the default data, plus it's custom data. This made a very large and repetitive multimocks.js file (around 7MB for our app and counting). The suggested was of splitting the multimocks file would have resulted in 124 files, which seems excessive. I could have done a lazy load the right mock file based on query param type of thing but assumed that would cause race conditions with Angular loading too fast.

By merging default with selected on the client side we can significantly reduce the size of the mock data (down to 1.1mb for us). This could potentially slow load of the client if there are a lot of requests to mock, as it's not a particularly efficient merge, however I'm pretty sure for most cases downloading a ~7MB file would be slower. There's also a bug in the current merge function https://github.com/wongatech/angular-multimocks/blob/master/tasks/multimocksGenerator.js#L27 should be:

return resource.scenario.uri + resource.scenario.httpMethod;

I'm happy to put the original way back and make it configurable (and change the template a bit so the client side knows about it) as I realise I made a fairly big assumption that you'd want this, but it's easier to explain by having a diff :-)

I had trouble running the tests due to old versions of Karma/Phantom so updated them. Also don't have Grunt installed globally so just added a couple of wrapper npm commands.

Hope you like!

nabil-boag commented 7 years ago

Wow! Thanks for the PR.

I have pulled your code down and tried to run tests but I am having an issue with JSHint with your changes. I know that you didn't change anything to do with JSHint, I think that its just that the version has been bumped as some point.

This is the issue:

https://github.com/jshint/jshint/issues/2922

I tried adding an empty reporterOutput option and it fixes the issue, can you please add that to your PR.

jshint: {
     ...
      options: {
        jshintrc: '.jshintrc',
        reporterOutput: ''
      }
    },

If you add that in I will merge your PR.

hbk619 commented 7 years ago

I ran into that too, have you run npm update? I bumped the grunt-contrib-jshint version to 1.1.0 which pulls down jshint 2.9.4

If you've got those versions and still having issues I'll add the extra reporter option :-) Thanks for the quick response!

nabil-boag commented 7 years ago

Hey!

I did a fresh install from a Ubuntu VM and had the JSHint issue so please add that fix in.

After the install I did get the versions you said above but still had the issue.

hbk619 commented 7 years ago

Added, I must have something lying around that's making it work, my bad.

nabil-boag commented 7 years ago

Hmm maybe its and ubuntu/npm thing. Travis didn't seem to have the problem either.

hbk619 commented 7 years ago

Very strange, if it helps I'm on El Capitan with node 6.9.1 and npm 3.10.3