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

feat(Store): Added injection token option for feature modules #153

Merged
merged 1 commit into from
Jul 25, 2017

Conversation

brandonroberts
Copy link
Member

  • Fixes AOT issues when providing reducers via an InjectionToken
  • Adds a build step before starting the example app
  • Adds the example-app to CI testing

Closes #116, #141, #147

@victornoel
Copy link

@brandonroberts I tried to use this branch to validate it did fix #147 with my code (I build it and replaced the folders in my node_modules/@ngrx with those from dist), and when I run my application with --prod, I get the following error:

ERROR in /home/victor/code/petals-cockpit/frontend/src/$$_gendir/node_modules/@ngrx/store/store.ngfactory.ts (51,65): Argument of type '{}' is not assignable to parameter of type 'StoreFeature<any, any>[]'.
  Property 'includes' is missing in type '{}'.

I suspect there is something wrong with this PR or is it me doing something wrong?

@victornoel
Copy link

using typescript 2.4, maybe that's why, I noticed you have both typescript 2.4 and 2.3 in your node_modules (2.4 at the root, 2.3 under angular-cli), and angular-cli will use 2.3 so the example app is most certainly built and executed with 2.3.

@victornoel
Copy link

I tried to execute the example-app with ts 2.4 and it works, so I suppose it is just that now #147 breaks at build time instead of runtime... (and thus this PR does not solve #147 :)

@victornoel
Copy link

even though it is strange because I don't use features at all in my application (see #147 (comment))

@bfricka
Copy link
Contributor

bfricka commented Jul 23, 2017

I have to admit, I tried implementing this the other day, and couldn't figure it out. Nice work.

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

4 participants