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

Add ReactViewModel #3319

Merged
merged 3 commits into from
Jul 5, 2017
Merged

Add ReactViewModel #3319

merged 3 commits into from
Jul 5, 2017

Conversation

christopherjbaker
Copy link

@christopherjbaker christopherjbaker commented Jun 19, 2017

I added the shrinkwrapped dependencies, modified the docs glob, and incorporated the tests.

@christopherjbaker
Copy link
Author

Current test failures are fixed in canjs/can-stache-converters#34

@christopherjbaker christopherjbaker changed the base branch from master to can-reflect June 23, 2017 17:56
@phillipskevin phillipskevin mentioned this pull request Jun 23, 2017
@christopherjbaker christopherjbaker force-pushed the can-reflect branch 11 times, most recently from 568b8b4 to 58e86d5 Compare June 29, 2017 22:04
@christopherjbaker christopherjbaker changed the base branch from can-reflect to master July 3, 2017 15:28
@justinbmeyer
Copy link
Contributor

@m-mujica / @matthewp it seems that process isn't provided anymore to the built files. Can you take a look at this?

@matthewp
Copy link
Contributor

matthewp commented Jul 4, 2017

Are you talking about what is loaded by test/index.html? That loads can.all.js, which is a global exported build. As far as I remember the global shim never had process, only the steal npm plugin (which does still have it). I'm not opposed to adding it to the global shim though.

@matthewp
Copy link
Contributor

matthewp commented Jul 5, 2017

I don't understand how this is supposed to work. There are several libs being ignored in the build: https://github.com/canjs/canjs/blob/master/build.js#L6-L7 . but there are no corresponding script tags for those libs in the text HTML pages. So I don't understand how those libs are not throwing.

@matthewp
Copy link
Contributor

matthewp commented Jul 5, 2017

Digging into it more it looks like, in the example of can-stream-kefir, that kefir is excluded from the build and is not on the window, but because can-stream-kefir doesn't care in the module body if Kefir is undefined, it doesn't break. But it would break if someone attempted to use it (although I'm not quite sure how they would do so).

@matthewp
Copy link
Contributor

matthewp commented Jul 5, 2017

So I think we need to either include react and all of its dependencies into the build or exclude it, and react-view-model and can-react-component. For now I'm not to exclude those.

@matthewp matthewp force-pushed the add-react-view-model branch from 1029884 to b6d73be Compare July 5, 2017 12:16
@matthewp matthewp merged commit d8a04a0 into master Jul 5, 2017
@matthewp matthewp deleted the add-react-view-model branch July 5, 2017 12:47
@justinbmeyer
Copy link
Contributor

justinbmeyer commented Jul 5, 2017 via email

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.

4 participants