react2angular icon indicating copy to clipboard operation
react2angular copied to clipboard

feat: add support for children transclude

Open spirosikmd opened this issue 8 years ago • 13 comments

Fixes #12

@bcherny I followed the recommendations mentioned in the issue. I created a prototype. Seems that it is possible. However, the last part I am not sure how to proceed best. Current status is:

<test-angular-two foo="foo" bar="bar" baz="baz" class="ng-scope ng-isolate-scope">
  <div data-reactroot="">
    <p><!-- react-text: 3 -->Foo: <!-- /react-text --><!-- react-text: 4 -->1<!-- /react-text --></p>
    <p><!-- react-text: 6 -->Bar: <!-- /react-text --><!-- react-text: 7 -->true,false<!-- /react-text --></p>
    <p>Baz</p>
    <!-- react-text: 9 -->&lt;span className="ng-scope"&gt;Transcluded&lt;/span&gt;<!-- /react-text -->
    <!-- react-text: 10 -->&lt;p className="ng-scope"&gt;Transcluded&lt;/p&gt;<!-- /react-text -->
  </div>
</test-angular-two>

the transcluded components are added as children, but they are escaped. Any idea? Maybe use angular $sce?

spirosikmd avatar Sep 05 '17 22:09 spirosikmd

So @bcherny I managed to fix the escaping issue by using html-react-parser! I think this is ready for review. Let me know what you think?

spirosikmd avatar Sep 06 '17 14:09 spirosikmd

@spirosikmd This is excellent, thanks for that hard work! A couple of notes:

  • Can you please add tests for updating the scope when transcluded elements use stuff from that scope (making sure the transcluded content updates, isn't double-rendered, etc.)
  • How much do the extra dependencies add to the build size?

bcherny avatar Sep 06 '17 18:09 bcherny

Hi @bcherny! Thank you for providing the package in the first place!

The sizes for support-transclude branch are:

577B index.d.ts
2.0K index.es2015.js
2.3K index.js
1.5K index.js.map

compared to master branch:

577B index.d.ts
1.4K index.es2015.js
2.4K index.js
889B index.js.map

Size of index.js hasn't increased at all. However, index.es2015.js size increased by ~0.6K. Do you think this is would be a problem?

spirosikmd avatar Sep 06 '17 19:09 spirosikmd

@spirosikmd 0.6kb is no big deal, but what happens when you run that through browserify (to pull in htmltojsx, html-react-parser, and all of their dependencies)?

bcherny avatar Sep 06 '17 19:09 bcherny

When running the following command

./node_modules/.bin/browserify index.js -o bundle.js

on both branches, the bundle.js file ends up being 2.5M for support-transclude and 1.1M for master. Big difference I think. How to improve that?

spirosikmd avatar Sep 06 '17 19:09 spirosikmd

I was worried it would be more in the 10-20mb range. 2.5mb is fine I think. A tree-shaking compiler (rollup, webpack) will improve that size, and we get a lot of functionality for that extra 1.4mb.

bcherny avatar Sep 06 '17 19:09 bcherny

Cool ok then! I will work on adding some tests and docs for this feature.

spirosikmd avatar Sep 06 '17 20:09 spirosikmd

@spirosikmd Any updates on this?

bcherny avatar Oct 22 '17 15:10 bcherny

Hey @bcherny! I haven't worked on this yet. I am trying the approach described in this article, passing all the data needed as props and don't use transclusion.

spirosikmd avatar Oct 24 '17 07:10 spirosikmd

@spirosikmd Sounds good! I was doing some PR cleanup and wanted to make sure this wasn't forgotten.

bcherny avatar Oct 24 '17 20:10 bcherny

Any update?

andreoav avatar Mar 05 '18 22:03 andreoav

Any update?

gairon avatar Dec 29 '18 13:12 gairon

Hi @andreoav @gairon! I stopped working on this. I won't invest in angular.js anymore. I see @gregbty is working on a similar PR. He runs into the same issues I was running as well.

spirosikmd avatar Dec 29 '18 13:12 spirosikmd