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

fix get children dependency bug #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

s97712
Copy link

@s97712 s97712 commented Jun 22, 2019

No description provided.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 28

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 27: 0.0%
Covered Lines: 226
Relevant Lines: 226

💛 - Coveralls

@gnaeus
Copy link
Owner

gnaeus commented Jul 30, 2019

Sorry for a long wait. If I clearly understand, your PR allows to register dependencies of parent component services inside child component providers. But it can results to unpredictable behaviour.

For example... Suppose we have an SPA with two pages: Home and About. And two services: AppService and PageService.

class PageService {
  dispose() {
    // ...
  }
}

class AppService {
  @inject pageService: PageService;
}

We register AppService in App component level.

@provider(AppService)
class App extends React.Component {
  render() {
    return (
      <Router>
        <Route path="/home" component={Home} />
        <Route path="/about" component={About} />
      </Router>
    )
  }
}

Then we register PageService in concrete page's provider. And inject AppService into each page.

@provider(PageService)
class Home extends React.Component {
  @inject appService: AppService;
  // ...
}

@provider(PageService)
class About extends React.Component {
  @inject appService: AppService;
  // ...
}

But wait... If user starts our SPA from /home route, then appService lives in App provider and appService.pageService in Home provider. But if user starts from /about route, appService.pageService will be stored in About provider. So our initialization logic implicitly depends on route visiting sequence.

Moreover, if user navigates from /home to /about then Home page provider will be unmounted. And appService.pageService will be disposed by calling pageService.dispose() inside react-ioc.

The right way is to move up PageService registration to App component provider.

@provider(AppService, PageService)
class App extends React.Component // ...

@s97712
Copy link
Author

s97712 commented Aug 2, 2019

Thank for your reply.
Sub-dependency is very common and useful in angular. I really like this feature, and will try to fix the problem.

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

3 participants