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

Deservicify core #2458

Merged
merged 7 commits into from Jul 7, 2019

Conversation

@isiahmeadows
Copy link
Collaborator

commented Jul 6, 2019

Edit: Updated with current info

Description

I merged the implementations of m.mount and m.redraw, I removed the intermediate route service from m.route, and I put m.mount/m.redraw in charge of actual throttling as it simplified a lot. I also reduced the scope of the dependencies being injected, so it's a little easier to test.

Oh, and this makes it pretty clear that m.route could very easily be implemented in userland - it literally just depends on m.mount and m.redraw. Down this vein, I then updated render to accept a new redraw callback as a third parameter and moved m.mount/m.redraw to be implemented in terms of that, making that entirely agnostic of renderer.

This reduces the bundle size to 9400 bytes min+gzip, where it was previously 9560. Small win, but that's not the main benefit of this.

Note that this now no longer exposes redrawService from mithril/redraw and instead just exports the redraw callback directly. So if you were previously using mithril/redraw, here's how you need to migrate:

  • var redraw = require("mithril/redraw").redrawvar redraw = require("mithril/redraw") (drop the property access)
  • redrawService.schedule + redrawService.unschedule → Use components as necessary. If you need to do this away from the DOM for some reason, render to an element not kept in the live tree.
  • redrawService.render → In general, this is the wrong thing to do. Just use m.redraw() + components as appropriate.

Motivation and Context

This addresses a few long-standing issues us maintainers have talked about privately for a little over a year, and has been historically low-priority. It should also be a little easier to make sense of.

Fixes #2074 (requirement for #1907)

How Has This Been Tested?

I moved around a lot of tests and removed a couple duplicates.

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)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated docs/change-log.md

@project-bot project-bot bot added this to Needs triage in Triage/bugs Jul 6, 2019

isiahmeadows added some commits Jul 6, 2019

De-servicify router (mostly)
Still uses the redraw service, but it no longer has an intermediate
service of its own.

Also, did a *lot* of test deduplication in this. About 30-40% of the
router service tests were already tested on the main router API instance
itself.

Bundle size decreased from 9560 to 9548 bytes min+gzip.
Merge `m.mount` + `m.redraw`, update router
Simplifies the router and redraw mechanism, and makes it much easier to
keep predictable.

Bundle size down to 9433 bytes min+gzip, docs updated accordingly.
Deservicify `m.render`, revise `m.route`
- You now have to use `mithril/render/render` directly if you want an
  implicit redraw function. (This will likely be going away in v3.)
- Revise `m.route` to only `key` components

@isiahmeadows isiahmeadows force-pushed the isiahmeadows:deservicify branch from 755ddb1 to 35a34eb Jul 7, 2019

@isiahmeadows isiahmeadows marked this pull request as ready for review Jul 7, 2019

@isiahmeadows isiahmeadows merged commit 1f4b2cf into MithrilJS:next Jul 7, 2019

Triage/bugs automation moved this from Needs triage to Closed Jul 7, 2019

@isiahmeadows isiahmeadows deleted the isiahmeadows:deservicify branch Jul 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
1 participant
You can’t perform that action at this time.