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.

Sign up for GitHub

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

CPLAT-4460: Make compatible for use with react.dart 5.0.0-wip #255

Merged
merged 8 commits into from
Mar 7, 2019
Merged

CPLAT-4460: Make compatible for use with react.dart 5.0.0-wip #255

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
173a7d0
remove outdated tests and update findDomNode to use try/catch
kealjones-wk Feb 28, 2019
aa61dde
non ssh git ref
kealjones-wk Feb 28, 2019
1d5e08b
Remove dependency_overrides
kealjones-wk Mar 1, 2019
591f9dd
Ill just take this back...
kealjones-wk Mar 1, 2019
17aea03
Address CR Feedback and fix README
kealjones-wk Mar 6, 2019
9eb3ede
Address CR Feedback
kealjones-wk Mar 6, 2019
cc55e4d
re-add string ref tests
kealjones-wk Mar 7, 2019
7e0ca51
fix indentation
kealjones-wk Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ and add an HTML element with a unique identifier where you’ll mount your OverR
```html
<html>
<head>
<!-- ... -->
<!-- ... -->
</head>
<body>
<div id="react_mount_point">
Expand All @@ -68,21 +68,21 @@ and add an HTML element with a unique identifier where you’ll mount your OverR

<script src="packages/react/react.js"></script>
<script src="packages/react/react_dom.js"></script>

<script type="application/javascript" defer src="your_app_entrypoint.dart.js"></script>
</body>
</html>
```

> __Note:__ When serving your application in production, use `packages/react/react_with_react_dom_prod.js`
file instead of the un-minified `react.js` / `react_dom.js` files shown in the example above.
file instead of the un-minified `react.js` / `react_dom.js` files shown in the example above.

4. Import the `over_react` and `react_dom` libraries into `your_app_name.dart`, and initialize
React within your Dart application. Then [build a custom component](#building-custom-components) and
mount / render it into the HTML element you created in step 3.

> Be sure to namespace the `react_dom.dart` import as `react_dom` to avoid collisions with `UiComponent.render`
when [creating custom components](#building-custom-components).
when [creating custom components](#building-custom-components).

```dart
import 'dart:html';
Expand All @@ -95,7 +95,7 @@ mount / render it into the HTML element you created in step 3.

// Mount / render your component.
react_dom.render(Foo()(), querySelector('#react_mount_point'));
}
}
```

5. Run `pub run build_runner serve` in the root of your Dart project.
Expand All @@ -109,24 +109,24 @@ properly. Unfortunately, this is a known limitation in the analysis server at th

When running tests on code that uses our [builder] _(or any code that imports `over_react`)_,
__you must run your tests using build_runner__.
>**Warning:** Do **_not_** run tests via `pub run build_runner test` in a package while another instance of `build_runner`
>**Warning:** Do **_not_** run tests via `pub run build_runner test` in a package while another instance of `build_runner`
(e.g. `pub run build_runner serve`)is running in that same package. [This workflow is unsupported by build_runner](https://github.com/dart-lang/build/issues/352#issuecomment-461554316)

1. Run tests through build_runner, and specify the platform to be a browser platform. Example:
1. Run tests through build_runner, and specify the platform to be a browser platform. Example:

```bash
$ pub run build_runner test -- -p chrome test/your_test_file.dart
```

1. When running tests in `over_react`, our `dart_test.yaml` specifies some handy presets for running tests in DDC and dart2js:
> **Note:** These presets exist only in `over_react`.
* To run tests in `over_react` compiled via DDC, run:
```bash
$ pub run build_runner -- -P dartdevc
$ pub run build_runner test -- -P dartdevc
```
* To run tests in `over_react` compiled via dart2js, run:
```bash
$ pub run build_runner -r -- -P dart2js
$ pub run build_runner test -r -- -P dart2js
```

&nbsp;
Expand All @@ -145,7 +145,7 @@ The `over_react` library functions as an additional "layer" atop the [Dart react
which handles the underlying JS interop that wraps around [React JS][react-js].

The library strives to maintain a 1:1 relationship with the React JS component class and API.
To do that, an OverReact component is comprised of four core pieces that are each wired up
To do that, an OverReact component is comprised of four core pieces that are each wired up
via our builder using an analogous [annotation].

1. [UiFactory](#uifactory)
Expand Down Expand Up @@ -184,7 +184,7 @@ class _$FooProps extends UiProps {
```
* Note: The [builder] will make the concrete getters and setters available in a generated class which has the same name
as the class annotated with `@Props()`, but without the `_$` prefix (which would be `FooProps` in the above code).
The generated class will also have the same API. So, consumers who wish to extend the functionality of `_$FooProps` should
The generated class will also have the same API. So, consumers who wish to extend the functionality of `_$FooProps` should
extend the generated version, `FooProps`.

&nbsp;
Expand All @@ -202,7 +202,7 @@ class _$FooProps extends UiProps {

@Component()
class FooComponent extends UiComponent<FooProps> {
// ...
// ...
}

void bar() {
Expand Down Expand Up @@ -285,7 +285,7 @@ class _$FooState extends UiState {
> UiState is optional, and won’t be used for every component.
* Note: The [builder] will make the concrete getters and setters available in a generated class which has the same name
as the class annotated with `@State()`, but without the `_$` prefix (which would be `FooState` in the above code).
The generated class will also have the same API. So, consumers who wish to extend the functionality of `_$FooState` should
The generated class will also have the same API. So, consumers who wish to extend the functionality of `_$FooState` should
use the generated version, `FooState`.

&nbsp;
Expand Down Expand Up @@ -662,7 +662,7 @@ Now that we’ve gone over how to [use the `over_react` package in your project]
the [anatomy of a component](#anatomy-of-an-overreact-component) and the [DOM components](#dom-components-and-props)
that you get for free from OverReact, you're ready to start building your own custom React UI components.

1. Start with one of the [component boilerplate templates](#component-boilerplate-templates) below
1. Start with one of the [component boilerplate templates](#component-boilerplate-templates) below
(Or, use OverReact's [code snippets for Intellij and Vs Code](https://github.com/Workiva/over_react/blob/master/snippets/README.md)).
* [Component](#component-boilerplate) _(props only)_
* [Stateful Component](#stateful-component-boilerplate) _(props + state)_
Expand Down
42 changes: 21 additions & 21 deletions lib/src/util/react_wrappers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,27 +260,27 @@ T getDartComponent<T extends react.Component>(/* ReactElement|ReactComponent|Ele
// in the test output when running `ddev test`.
print(unindent(
'''
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* WARNING: `getDartComponent`
*
* react-dart 4.0 no longer supports retrieving Dart components from
* `ReactElement` (pre-mounted VDOM instances) in order to prevent memory leaks.
*
* This call to `getDartComponent` will return `null`.
*
* Start using the mounted JS component instance instead:
*
* Example:
* // Before
* var vdom = Button()('Click me');
* react_dom.render(vdom, mountNode);
* var dartInstance = getDartComponent(vdom);
*
* // Fixed
* var vdom = Button()('Click me');
* var jsInstance = react_dom.render(vdom, mountNode);
* var dartInstance = getDartComponent(jsInstance);
*
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* WARNING: `getDartComponent`
*
* react-dart 4.0 no longer supports retrieving Dart components from
* `ReactElement` (pre-mounted VDOM instances) in order to prevent memory leaks.
*
* This call to `getDartComponent` will return `null`.
*
* Start using the mounted JS component instance instead:
*
* Example:
* // Before
* var vdom = Button()('Click me');
* react_dom.render(vdom, mountNode);
* var dartInstance = getDartComponent(vdom);
*
* // Fixed
* var vdom = Button()('Click me');
* var jsInstance = react_dom.render(vdom, mountNode);
* var dartInstance = getDartComponent(jsInstance);
*
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
'''
));
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,5 @@ dev_dependencies:
dart_dev: ^2.0.0
dependency_validator: ^1.2.2
mockito: ^4.0.0
over_react_test: ^2.2.0
over_react_test: ^2.4.0
test: ^1.0.0
12 changes: 1 addition & 11 deletions test/over_react/component_declaration/component_base_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,15 +463,6 @@ main() {
component.unwrappedProps = {};
});

group('`ref`', () {
test('should return a reference to the string ref', () {
var renderedInstance = render(TestComponent()());
TestComponentComponent component = getDartComponent(renderedInstance);

expect(component.ref('foo'), isNotNull);
});
});

group('`props`', () {
group('getter:', () {
test('returns a UiProps view into the component\'s props map', () {
Expand Down Expand Up @@ -947,7 +938,6 @@ dynamic getDartChildren(var renderedInstance) {
return getProps(renderedInstance)['children'];
}


UiFactory<TestComponentProps> TestComponent = ([Map props]) => new TestComponentProps(props);

class TestComponentProps extends UiProps {
Expand All @@ -965,7 +955,7 @@ class TestComponentComponent extends UiComponent<TestComponentProps> {
TestComponentComponent({List<ConsumedProps> testConsumedProps}) : consumedProps = testConsumedProps;

@override
render() => (Dom.div()..ref = 'foo')();
render() => Dom.div()();

@override
TestComponentProps typedPropsFactory(Map propsMap) => new TestComponentProps(propsMap);
Expand Down
5 changes: 0 additions & 5 deletions test/over_react/dom/render_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ main() {
});

group('throws', () {
test('when `element` is `null`', () {
expect(() => react_dom.render(null, mountNode), throwsA(anything));
expect(mountNode.children, isEmpty);
});

test('when `mountNode` is `null`', () {
expect(() => react_dom.render(Dom.div()('oh hai'), null), throwsA(anything));
});
Expand Down
17 changes: 12 additions & 5 deletions test/over_react/util/react_wrappers_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,16 +227,23 @@ main() {
});

group('updates the "key" and "ref" props properly', () {
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
const Map originalKeyRefProps = const {
var originalRefCalled = false;
var cloneRefCalled = false;
Map originalKeyRefProps = {
'key': 'original',
'ref': 'original'
'ref': (ref) { originalRefCalled = true; }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were changed to callback refs because string refs are considered legacy and will be deprecated eventually

};

const Map overrideKeyRefProps = const {
Map overrideKeyRefProps = {
'key': 'clone',
'ref': 'clone'
'ref': allowInterop((ref) { cloneRefCalled = true; })
};

tearDown((){
originalRefCalled = false;
cloneRefCalled = false;
});

test('for a plain React JS component', () {
var original = (Dom.div()..addProps(originalKeyRefProps))(testChildren);
var clone = cloneElement(original, overrideKeyRefProps);
Expand Down Expand Up @@ -264,7 +271,7 @@ main() {

// Verify that "key" and "ref" are overridden according to React
expect(clone.key, equals(overrideKeyRefProps['key']));
expect(clone.ref, equals(overrideKeyRefProps['ref']));
expect(cloneRefCalled, isTrue);

var renderedClone = react_test_utils.findRenderedComponentWithTypeV2(renderedHolder, TestComponentFactory);

Expand Down