Skip to content
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

Refactored instrumentClientJs #68

Closed

Conversation

SimonSimCity
Copy link
Contributor

Refactored the instrumentClientJs action to mock the request and inject the instruments to the content.

Description

This way leaves the finding of data off to the actual handler and just tip in when the response-object receives data. This data then is decompressed if necessary and the instruments are injected.

Motivation and Context

The main motivation was the rewrite of the package dynamic-import which (by an update to Meteor 1.6.1) has switched over to use HTTP requests instead of a Meteor method to request the new data. This way here provides a clean way to hook on to this method and inject the instruments.

I haven't included a parser yet that would parse the /__meteor__/dynamic-import/fetch requests. I'll do this as soon as I got my project running on Meteor 1.6.1 but this is a huge step forward to this, I believe.

See: https://github.com/meteor/meteor/blob/devel/packages/dynamic-import

How Has This Been Tested?

Start the coverage on the client and confirm that the js-files build up the coverage-report

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

@serut
Copy link
Owner

serut commented May 9, 2018

Can you rebase this PR on the current master ? To see if tests are ko or good.

@SimonSimCity
Copy link
Contributor Author

I'd like to close this in favor to #73 which is a better way and actually solves the problem of #63

@serut serut closed this May 10, 2018
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.

None yet

2 participants