From a169a8c08ec72e40f3881bad8808fdd47e61c379 Mon Sep 17 00:00:00 2001 From: PSWai Date: Wed, 11 Jul 2018 16:06:09 +0800 Subject: [PATCH 1/9] Add support to have ReactStyleFileName --- addon/resolver.js | 27 +++++++++++++++++-- .../app/components/ReactStyleFileName.jsx | 7 +++++ .../components/react-component-test.js | 20 ++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 tests/dummy/app/components/ReactStyleFileName.jsx diff --git a/addon/resolver.js b/addon/resolver.js index d93231d..ba0c7b7 100644 --- a/addon/resolver.js +++ b/addon/resolver.js @@ -6,26 +6,49 @@ import ReactComponent from 'ember-cli-react/components/react-component'; const { get } = Ember; export default Resolver.extend({ + // `resolveComponent` is triggered when rendering a component in template. + // For example, having `{{foo-bar}}` in a template will trigger `resolveComponent` + // with the name full name of `component:foo-bar`. resolveComponent(parsedName) { - const result = this.resolveOther(parsedName); + // First try to resolve with the convention of Ember CLI via `resolveOther`. + // If nothing is found, try again with React-styled file name (e.g. SayHi). + let result = + this.resolveOther(parsedName) || this.resolveReactStyleFile(parsedName); + // If there is no result found after all, return nothing if (!result) { return; } + // If there is an Ember component found, return it. + // This includes the `react-component` Ember component. if (get(result, 'isComponentFactory')) { return result; } else { + // This enables using React Components directly in template return ReactComponent.extend({ reactComponent: result, }); } }, + // This resolver method is defined when we try to lookup from `react-component`. + // We create a new namespace `react-component:the-component` for them. resolveReactComponent(parsedName) { parsedName.type = 'component'; - const result = this.resolveOther(parsedName); + const result = + this.resolveOther(parsedName) || this.resolveReactStyleFile(parsedName); parsedName.type = 'react-component'; return result; }, + + // This resolver method attempt to find a file with React-style file name. + // A React-style file name is capitalized camel-cased. + resolveReactStyleFile(parsedName) { + const originalName = parsedName.fullNameWithoutType; + parsedName.fullNameWithoutType = Ember.String.classify(originalName); + const result = this.resolveOther(parsedName); + parsedName.fullNameWithoutType = originalName; + return result; + }, }); diff --git a/tests/dummy/app/components/ReactStyleFileName.jsx b/tests/dummy/app/components/ReactStyleFileName.jsx new file mode 100644 index 0000000..261550a --- /dev/null +++ b/tests/dummy/app/components/ReactStyleFileName.jsx @@ -0,0 +1,7 @@ +import React from 'npm:react'; + +const ReactStyleFileName = () => { + return My file name is ReactStyleFileName; +}; + +export default ReactStyleFileName; diff --git a/tests/integration/components/react-component-test.js b/tests/integration/components/react-component-test.js index e9c596c..0e9a360 100644 --- a/tests/integration/components/react-component-test.js +++ b/tests/integration/components/react-component-test.js @@ -323,6 +323,26 @@ describeComponent( expect(this.$().text()).to.contain('Hello Morgan'); }); + + it('supports React-style component file name', function() { + this.render(hbs`{{react-component "react-style-file-name"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is ReactStyleFileName'); + }); + + it('supports React-style component file name when rendering directly', function() { + this.render(hbs`{{react-style-file-name}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is ReactStyleFileName'); + }); }); } ); From 625179f99cfd04552ea85e816f6aa02109899f99 Mon Sep 17 00:00:00 2001 From: PSWai Date: Wed, 11 Jul 2018 16:34:20 +0800 Subject: [PATCH 2/9] Add test for single-worded component --- tests/dummy/app/components/Card.jsx | 7 +++++++ tests/integration/components/react-component-test.js | 10 ++++++++++ 2 files changed, 17 insertions(+) create mode 100644 tests/dummy/app/components/Card.jsx diff --git a/tests/dummy/app/components/Card.jsx b/tests/dummy/app/components/Card.jsx new file mode 100644 index 0000000..2f970ad --- /dev/null +++ b/tests/dummy/app/components/Card.jsx @@ -0,0 +1,7 @@ +import React from 'npm:react'; + +const Card = () => { + return I am a Card component, I have no dash!; +}; + +export default Card; diff --git a/tests/integration/components/react-component-test.js b/tests/integration/components/react-component-test.js index 0e9a360..d94966b 100644 --- a/tests/integration/components/react-component-test.js +++ b/tests/integration/components/react-component-test.js @@ -334,6 +334,16 @@ describeComponent( ).to.equal('My file name is ReactStyleFileName'); }); + it('supports React-style component file name even without dash', function() { + this.render(hbs`{{react-component "card"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('I am a Card component, I have no dash!'); + }); + it('supports React-style component file name when rendering directly', function() { this.render(hbs`{{react-style-file-name}}`); From 31aee7704469f198246fc2f8defa4fb654929b82 Mon Sep 17 00:00:00 2001 From: PSWai Date: Wed, 11 Jul 2018 16:34:46 +0800 Subject: [PATCH 3/9] Update Readme on ReactStyleFileName --- README.md | 56 ++++++++++++++++++++++++++++++++++++++++++++-------- package.json | 2 +- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 3a53895..0f47812 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,10 @@ **Experimental Addon** -This was built as a prototype to evaluate using react inside of our Ember apps. We are not yet using it in production. PRs and constructive questions and comments via [GitHub issues](https://github.com/AltSchool/ember-cli-react/issues/new) are highly encouraged. +This was built as a prototype to evaluate using react inside of our Ember apps. +We are not yet using it in production. PRs and constructive questions and +comments via [GitHub +issues](https://github.com/AltSchool/ember-cli-react/issues/new) are highly +encouraged. # ember-cli-react @@ -53,7 +57,8 @@ Then render your component in a handlebars template: ## Block Form -Your React component can be used in block form to allow composition with existing Ember or React components. +Your React component can be used in block form to allow composition with +existing Ember or React components. ```handlebars {{#react-panel}} @@ -63,8 +68,9 @@ Your React component can be used in block form to allow composition with existin The children of `react-panel` will be populated to `props.children`. -Note that if the children contains mutating structure (e.g. `{{if}}`, `{{each}}`), -you need to wrap them in a stable tag to work around [this Glimmer issue](https://github.com/yapplabs/ember-wormhole/issues/66#issuecomment-263575168). +Note that if the children contains mutating structure (e.g. `{{if}}`, +`{{each}}`), you need to wrap them in a stable tag to work around [this Glimmer +issue](https://github.com/yapplabs/ember-wormhole/issues/66#issuecomment-263575168). ```handlebars {{#react-panel}} @@ -78,8 +84,39 @@ you need to wrap them in a stable tag to work around [this Glimmer issue](https: {{/react-panel}} ``` -Although this is possible, block form should be used as a tool to migrate Ember to React -without the hard requirement to start with leaf components. It is highly recommended to have clean React component tree whenever possible for best performance. +Although this is possible, block form should be used as a tool to migrate Ember +to React without the hard requirement to start with leaf components. It is +highly recommended to have clean React component tree whenever possible for best +performance. + +## Using File Name Convention for React + +React is unopinionated with file name convention. However, the majority of the +community has still developed some conventions over time. + +For React component files, the widely adopted convention is PascalCase, +including +[Airbnb](https://github.com/airbnb/javascript/tree/master/react#naming). So we +have added support for this convention. + +```handlebars +{{!-- Either `user-avatar.jsx` or `UserAvatar.jsx` works --}} +{{user-avatar}} +``` + +### Single-worded Component + +Ember requires at least a dash for component names. So single-worded component +(e.g. `Avatar`) cannot be used directly in Handlebars. However, you can still use +single-worded component with `react-component` component. + +```handlebars +{{!-- This won't work because Ember requires a dash for component --}} +{{avatar}} + +{{!-- This works --}} +{{react-component 'avatar'}} +``` ## Mini Todo List Example @@ -176,6 +213,9 @@ export default class TodoItem extends React.Component { ## What's Missing -There is no React `link-to` equivalent for linking to Ember routes inside of your React code. Instead pass action handlers that call `transitionTo` from an Ember route or component. +There is no React `link-to` equivalent for linking to Ember routes inside of +your React code. Instead pass action handlers that call `transitionTo` from an +Ember route or component. -In order to create minified production builds of React you must set `NODE_ENV=production`. +In order to create minified production builds of React you must set +`NODE_ENV=production`. diff --git a/package.json b/package.json index 88843a2..8818f72 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "eslint --fix", "git add" ], - "+(*.{json,css}|.prettierrc|.watchmanconfig)": [ + "+(*.{json,css,md}|.prettierrc|.watchmanconfig)": [ "prettier --write", "git add" ] From c5974bfa4d080ff6ee6f6fb107d5ec46304e34fe Mon Sep 17 00:00:00 2001 From: PSWai Date: Mon, 16 Jul 2018 14:21:33 +0800 Subject: [PATCH 4/9] Added various tests to confirm resolver priority --- tests/dummy/app/components/Card.jsx | 2 +- .../app/components/ReactStyleFileName.jsx | 2 +- tests/dummy/app/components/SameNameJsx.jsx | 7 ++ tests/dummy/app/components/same-name-jsx.jsx | 7 ++ .../components/react-component-test.js | 66 +++++++++++++++++++ 5 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 tests/dummy/app/components/SameNameJsx.jsx create mode 100644 tests/dummy/app/components/same-name-jsx.jsx diff --git a/tests/dummy/app/components/Card.jsx b/tests/dummy/app/components/Card.jsx index 2f970ad..b752bc5 100644 --- a/tests/dummy/app/components/Card.jsx +++ b/tests/dummy/app/components/Card.jsx @@ -1,4 +1,4 @@ -import React from 'npm:react'; +import React from 'react'; const Card = () => { return I am a Card component, I have no dash!; diff --git a/tests/dummy/app/components/ReactStyleFileName.jsx b/tests/dummy/app/components/ReactStyleFileName.jsx index 261550a..24afa8e 100644 --- a/tests/dummy/app/components/ReactStyleFileName.jsx +++ b/tests/dummy/app/components/ReactStyleFileName.jsx @@ -1,4 +1,4 @@ -import React from 'npm:react'; +import React from 'react'; const ReactStyleFileName = () => { return My file name is ReactStyleFileName; diff --git a/tests/dummy/app/components/SameNameJsx.jsx b/tests/dummy/app/components/SameNameJsx.jsx new file mode 100644 index 0000000..b017324 --- /dev/null +++ b/tests/dummy/app/components/SameNameJsx.jsx @@ -0,0 +1,7 @@ +import React from 'react'; + +const SameNameJsxWithPascalCase = () => { + return My file name is "SameNameJsx.jsx"; +}; + +export default SameNameJsxWithPascalCase; diff --git a/tests/dummy/app/components/same-name-jsx.jsx b/tests/dummy/app/components/same-name-jsx.jsx new file mode 100644 index 0000000..7e12e9c --- /dev/null +++ b/tests/dummy/app/components/same-name-jsx.jsx @@ -0,0 +1,7 @@ +import React from 'react'; + +const SameNameJsxWithSnakeCase = () => { + return My file name is "same-name-jsx.jsx"; +}; + +export default SameNameJsxWithSnakeCase; diff --git a/tests/integration/components/react-component-test.js b/tests/integration/components/react-component-test.js index d94966b..c2d262d 100644 --- a/tests/integration/components/react-component-test.js +++ b/tests/integration/components/react-component-test.js @@ -325,6 +325,16 @@ describeComponent( }); it('supports React-style component file name', function() { + this.render(hbs`{{react-component "ReactStyleFileName"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is ReactStyleFileName'); + }); + + it('supports React-style component file name, but render with Ember style name', function() { this.render(hbs`{{react-component "react-style-file-name"}}`); expect( @@ -353,6 +363,62 @@ describeComponent( .trim() ).to.equal('My file name is ReactStyleFileName'); }); + + describe('when there are two JSX files with the same name but different casing', function() { + it('prioritises Ember-cli-style file name (snake-case)', function() { + this.render(hbs`{{react-component "SameNameJsx"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is "same-name-jsx.jsx"'); + }); + + it('prioritises Ember-cli-style file name (snake-case) when render with Ember-style name', function() { + this.render(hbs`{{react-component "same-name-jsx"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is "same-name-jsx.jsx"'); + }); + + it('prioritises Ember-cli-style file name (snake-case) when rendering directly', function() { + this.render(hbs`{{same-name-jsx}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is "same-name-jsx.jsx"'); + }); + }); + + // The React file will overwrite Ember file as that's how Broccoli-React works. + // Skipping this to keep this in mind. + describe.skip('when the JSX file has same name with Ember component file but different extension', function() { + it('prioritises the Ember component', function() { + this.render(hbs`{{react-component "same-name-ember"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('I am an Ember component'); + }); + + it('prioritises the Ember component when rendering directly', function() { + this.render(hbs`{{same-name-ember}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('I am an Ember component'); + }); + }); }); } ); From 4bb4475b5c1d22074388c1ad0775447e853c5a71 Mon Sep 17 00:00:00 2001 From: PSWai Date: Mon, 16 Jul 2018 14:26:36 +0800 Subject: [PATCH 5/9] Resolver to copy `parsedName` instead of modifying --- addon/resolver.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/addon/resolver.js b/addon/resolver.js index ba0c7b7..d4bcfff 100644 --- a/addon/resolver.js +++ b/addon/resolver.js @@ -46,9 +46,10 @@ export default Resolver.extend({ // A React-style file name is capitalized camel-cased. resolveReactStyleFile(parsedName) { const originalName = parsedName.fullNameWithoutType; - parsedName.fullNameWithoutType = Ember.String.classify(originalName); - const result = this.resolveOther(parsedName); - parsedName.fullNameWithoutType = originalName; + const parsedNameWithPascalCase = Object.assign({}, parsedName, { + fullNameWithoutType: Ember.String.classify(originalName), + }); + const result = this.resolveOther(parsedNameWithPascalCase); return result; }, }); From 9737b94d0bacccfcae6e777586f535267d1ded32 Mon Sep 17 00:00:00 2001 From: PSWai Date: Mon, 16 Jul 2018 14:43:51 +0800 Subject: [PATCH 6/9] Add more details to README --- README.md | 59 +++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0f47812..964bf69 100644 --- a/README.md +++ b/README.md @@ -30,15 +30,16 @@ yarn add --dev ember-cli-react ember generate ember-cli-react ``` -**NOTE**: -`ember-cli-react` relies on a custom resolver to discover components. If you have -installed `ember-cli-react` with the standard way then you should be fine. Otherwise, you will need to manually update the first line of `app/resolver.js` to `import Resolver from 'ember-cli-react/resolver';`. +**NOTE**: `ember-cli-react` relies on a custom resolver to discover components. +If you have installed `ember-cli-react` with the standard way then you should be +fine. Otherwise, you will need to manually update the first line of +`app/resolver.js` to `import Resolver from 'ember-cli-react/resolver';`. ## Usage Write your React component as usual: -```javascript +```jsx // app/components/say-hi.jsx import React from 'react'; @@ -53,7 +54,8 @@ Then render your component in a handlebars template: {{say-hi name="Alex"}} ``` -**NOTE**: Currently, `ember-cli-react` recognizes React components with `.jsx` extension only. +**NOTE**: Currently, `ember-cli-react` recognizes React components with `.jsx` +extension only. ## Block Form @@ -99,16 +101,39 @@ including [Airbnb](https://github.com/airbnb/javascript/tree/master/react#naming). So we have added support for this convention. +In short, you can name your JSX files in `PascalCase`, in addition to +`snake-case`. + +```handlebars +{{!-- Both `user-avatar.jsx` and `UserAvatar.jsx` work --}} +{{user-avatar}} +``` + +### Rendering in Template + +When using the `react-component` component, referencing your React components +with `PascalCase` is also supported. However, due to the "at least one dash" +policy, it won't work if the component name is used directly. + ```handlebars -{{!-- Either `user-avatar.jsx` or `UserAvatar.jsx` works --}} +{{!-- OK! --}} +{{react-component "user-avatar"}} + +{{!-- OK! --}} +{{react-component "UserAvatar"}} + +{{!-- OK! --}} {{user-avatar}} + +{{!-- NOT OK! --}} +{{UserAvatar}} ``` ### Single-worded Component Ember requires at least a dash for component names. So single-worded component -(e.g. `Avatar`) cannot be used directly in Handlebars. However, you can still use -single-worded component with `react-component` component. +(e.g. `Avatar`) cannot be used directly in Handlebars. However, you can still +use single-worded component with `react-component` component. ```handlebars {{!-- This won't work because Ember requires a dash for component --}} @@ -118,6 +143,24 @@ single-worded component with `react-component` component. {{react-component 'avatar'}} ``` +### Fallback to `snake-case` file name + +Whenever there is a conflict, component files with Ember-style convention will +be used. + +Examples: + +- When both `SameName.jsx` and `same-name.jsx` exist, `same-name.jsx` will be + used. +- When both `SameName.jsx` and `same-name.js` (Ember) exist, `same-name.js` will + be used + +#### Known issue + +If an Ember component and a React component has a same name (`same-name.js` and +`same-name.jsx`), the file with `.js` extension will be overwritten with the +output of `same-name.jsx`. We are still looking at ways to resolve this. + ## Mini Todo List Example A more complete example which demonstrates how to handle actions from within From 584489bcf542cd996dd93e32546531a55d63a53c Mon Sep 17 00:00:00 2001 From: PSWai Date: Mon, 16 Jul 2018 15:21:12 +0800 Subject: [PATCH 7/9] Added lots of tests for edge cases --- README.md | 12 ++-- addon/resolver.js | 18 +++-- .../components/SameNameDifferentCaseMixed.jsx | 7 ++ .../components/namespace/InsideNamespace.jsx | 7 ++ .../same-name-different-case-mixed.js | 3 + .../same-name-different-case-mixed.hbs | 1 + .../components/react-component-test.js | 70 +++++++++++++++---- 7 files changed, 93 insertions(+), 25 deletions(-) create mode 100644 tests/dummy/app/components/SameNameDifferentCaseMixed.jsx create mode 100644 tests/dummy/app/components/namespace/InsideNamespace.jsx create mode 100644 tests/dummy/app/components/same-name-different-case-mixed.js create mode 100644 tests/dummy/app/templates/components/same-name-different-case-mixed.hbs diff --git a/README.md b/README.md index 964bf69..430918d 100644 --- a/README.md +++ b/README.md @@ -143,21 +143,21 @@ use single-worded component with `react-component` component. {{react-component 'avatar'}} ``` -### Fallback to `snake-case` file name +### React Components are Prioritised -Whenever there is a conflict, component files with Ember-style convention will +Whenever there is a conflict, component files with React-style convention will be used. Examples: -- When both `SameName.jsx` and `same-name.jsx` exist, `same-name.jsx` will be - used. -- When both `SameName.jsx` and `same-name.js` (Ember) exist, `same-name.js` will +- When both `SameName.jsx` and `same-name.jsx` exist, `SameName.jsx` will be + used +- When both `SameName.jsx` and `same-name.js` (Ember) exist, `SameName.jsx` will be used #### Known issue -If an Ember component and a React component has a same name (`same-name.js` and +If an Ember component and a React component has exactly the same name but different extension (`same-name.js` and `same-name.jsx`), the file with `.js` extension will be overwritten with the output of `same-name.jsx`. We are still looking at ways to resolve this. diff --git a/addon/resolver.js b/addon/resolver.js index d4bcfff..7f8c999 100644 --- a/addon/resolver.js +++ b/addon/resolver.js @@ -10,10 +10,10 @@ export default Resolver.extend({ // For example, having `{{foo-bar}}` in a template will trigger `resolveComponent` // with the name full name of `component:foo-bar`. resolveComponent(parsedName) { - // First try to resolve with the convention of Ember CLI via `resolveOther`. - // If nothing is found, try again with React-styled file name (e.g. SayHi). + // First try to resolve with React-styled file name (e.g. SayHi). + // If nothing is found, try again with original convention via `resolveOther`. let result = - this.resolveOther(parsedName) || this.resolveReactStyleFile(parsedName); + this.resolveReactStyleFile(parsedName) || this.resolveOther(parsedName); // If there is no result found after all, return nothing if (!result) { @@ -37,17 +37,23 @@ export default Resolver.extend({ resolveReactComponent(parsedName) { parsedName.type = 'component'; const result = - this.resolveOther(parsedName) || this.resolveReactStyleFile(parsedName); + this.resolveReactStyleFile(parsedName) || this.resolveOther(parsedName); parsedName.type = 'react-component'; return result; }, // This resolver method attempt to find a file with React-style file name. - // A React-style file name is capitalized camel-cased. + // A React-style file name is in PascalCase. resolveReactStyleFile(parsedName) { const originalName = parsedName.fullNameWithoutType; + + // Convert the compnent name while preserving namespaces + const parts = originalName.split('/'); + parts[parts.length - 1] = Ember.String.classify(parts[parts.length - 1]); + const newName = parts.join('/'); + const parsedNameWithPascalCase = Object.assign({}, parsedName, { - fullNameWithoutType: Ember.String.classify(originalName), + fullNameWithoutType: newName, }); const result = this.resolveOther(parsedNameWithPascalCase); return result; diff --git a/tests/dummy/app/components/SameNameDifferentCaseMixed.jsx b/tests/dummy/app/components/SameNameDifferentCaseMixed.jsx new file mode 100644 index 0000000..1917eb3 --- /dev/null +++ b/tests/dummy/app/components/SameNameDifferentCaseMixed.jsx @@ -0,0 +1,7 @@ +import React from 'react'; + +const SameNameDifferentCaseMixed = () => { + return My file name is "SameNameDifferentCaseMixed.jsx"; +}; + +export default SameNameDifferentCaseMixed; diff --git a/tests/dummy/app/components/namespace/InsideNamespace.jsx b/tests/dummy/app/components/namespace/InsideNamespace.jsx new file mode 100644 index 0000000..5f92cd3 --- /dev/null +++ b/tests/dummy/app/components/namespace/InsideNamespace.jsx @@ -0,0 +1,7 @@ +import React from 'react'; + +const InsideNamespace = () => { + return I am inside a namespace!; +}; + +export default InsideNamespace; diff --git a/tests/dummy/app/components/same-name-different-case-mixed.js b/tests/dummy/app/components/same-name-different-case-mixed.js new file mode 100644 index 0000000..a3e1d83 --- /dev/null +++ b/tests/dummy/app/components/same-name-different-case-mixed.js @@ -0,0 +1,3 @@ +import Ember from 'ember'; + +export default Ember.Component.extend(); diff --git a/tests/dummy/app/templates/components/same-name-different-case-mixed.hbs b/tests/dummy/app/templates/components/same-name-different-case-mixed.hbs new file mode 100644 index 0000000..11ab511 --- /dev/null +++ b/tests/dummy/app/templates/components/same-name-different-case-mixed.hbs @@ -0,0 +1 @@ +My file name is "same-name-different-case-mixed.js" \ No newline at end of file diff --git a/tests/integration/components/react-component-test.js b/tests/integration/components/react-component-test.js index c2d262d..67bb5c2 100644 --- a/tests/integration/components/react-component-test.js +++ b/tests/integration/components/react-component-test.js @@ -364,59 +364,103 @@ describeComponent( ).to.equal('My file name is ReactStyleFileName'); }); - describe('when there are two JSX files with the same name but different casing', function() { - it('prioritises Ember-cli-style file name (snake-case)', function() { + it('supports React-style component file name even when namespaced', function() { + this.render(hbs`{{namespace/inside-namespace}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('I am inside a namespace!'); + }); + + describe('when both `SameNameJsx.jsx` and `same-name-jsx.jsx` exist', function() { + it('prioritises React-style file name (SameNameJsx.jsx)', function() { this.render(hbs`{{react-component "SameNameJsx"}}`); expect( this.$() .text() .trim() - ).to.equal('My file name is "same-name-jsx.jsx"'); + ).to.equal('My file name is "SameNameJsx.jsx"'); }); - it('prioritises Ember-cli-style file name (snake-case) when render with Ember-style name', function() { + it('prioritises React-style file name (SameNameJsx.jsx) when render with Ember-style name', function() { this.render(hbs`{{react-component "same-name-jsx"}}`); expect( this.$() .text() .trim() - ).to.equal('My file name is "same-name-jsx.jsx"'); + ).to.equal('My file name is "SameNameJsx.jsx"'); }); - it('prioritises Ember-cli-style file name (snake-case) when rendering directly', function() { + it('prioritises React-style file name (SameNameJsx.jsx) when rendering directly', function() { this.render(hbs`{{same-name-jsx}}`); expect( this.$() .text() .trim() - ).to.equal('My file name is "same-name-jsx.jsx"'); + ).to.equal('My file name is "SameNameJsx.jsx"'); + }); + }); + + describe('when both `SameNameDifferentCaseMixed.jsx` and `same-name-different-case-mixed.js` (Ember) exist', function() { + it('prioritises the React component (SameNameDifferentCaseMixed.jsx)', function() { + this.render(hbs`{{react-component "SameNameDifferentCaseMixed"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is "SameNameDifferentCaseMixed.jsx"'); + }); + + it('prioritises the React component (SameNameDifferentCaseMixed.jsx) when render with Ember-style name', function() { + this.render( + hbs`{{react-component "same-name-different-case-mixed"}}` + ); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is "SameNameDifferentCaseMixed.jsx"'); + }); + + it('prioritises the React component (SameNameDifferentCaseMixed.jsx) when rendering directly', function() { + this.render(hbs`{{same-name-different-case-mixed}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('My file name is "SameNameDifferentCaseMixed.jsx"'); }); }); // The React file will overwrite Ember file as that's how Broccoli-React works. // Skipping this to keep this in mind. - describe.skip('when the JSX file has same name with Ember component file but different extension', function() { - it('prioritises the Ember component', function() { - this.render(hbs`{{react-component "same-name-ember"}}`); + describe.skip('when both `same-name-same-case-mixed.jsx` and `same-name-same-case-mixed.js` (Ember) exist', function() { + it('prioritises the React component (same-name-same-case-mixed.js)', function() { + this.render(hbs`{{react-component "same-name-same-case-mixed"}}`); expect( this.$() .text() .trim() - ).to.equal('I am an Ember component'); + ).to.equal('My file name is "same-name-same-case-mixed.jsx"'); }); - it('prioritises the Ember component when rendering directly', function() { + it('prioritises the React component (same-name-same-case-mixed.js) when rendering directly', function() { this.render(hbs`{{same-name-ember}}`); expect( this.$() .text() .trim() - ).to.equal('I am an Ember component'); + ).to.equal('My file name is "same-name-same-case-mixed.jsx"'); }); }); }); From 8911fcad505adda7d45929d4fcd07f9052ad9d09 Mon Sep 17 00:00:00 2001 From: PSWai Date: Mon, 16 Jul 2018 15:40:07 +0800 Subject: [PATCH 8/9] Add one more test for namespaced edge case --- README.md | 4 ++-- tests/integration/components/react-component-test.js | 12 +++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 430918d..bf56ad3 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ **Experimental Addon** -This was built as a prototype to evaluate using react inside of our Ember apps. +This was built as a prototype to evaluate using React inside of our Ember apps. We are not yet using it in production. PRs and constructive questions and comments via [GitHub issues](https://github.com/AltSchool/ember-cli-react/issues/new) are highly @@ -140,7 +140,7 @@ use single-worded component with `react-component` component. {{avatar}} {{!-- This works --}} -{{react-component 'avatar'}} +{{react-component 'Avatar'}} ``` ### React Components are Prioritised diff --git a/tests/integration/components/react-component-test.js b/tests/integration/components/react-component-test.js index 67bb5c2..2148d28 100644 --- a/tests/integration/components/react-component-test.js +++ b/tests/integration/components/react-component-test.js @@ -364,7 +364,17 @@ describeComponent( ).to.equal('My file name is ReactStyleFileName'); }); - it('supports React-style component file name even when namespaced', function() { + it('supports React-style component file name with namespace', function() { + this.render(hbs`{{react-component "namespace/InsideNamespace"}}`); + + expect( + this.$() + .text() + .trim() + ).to.equal('I am inside a namespace!'); + }); + + it('supports React-style component file name when rendering directly with namespace', function() { this.render(hbs`{{namespace/inside-namespace}}`); expect( From 3c3d6cced4d704a3da9a2e4320af79a28193b133 Mon Sep 17 00:00:00 2001 From: PSWai Date: Tue, 17 Jul 2018 14:09:09 +0800 Subject: [PATCH 9/9] Rename `resolveReactStyleFile` to `_resolveReactStyleFile` --- addon/resolver.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/addon/resolver.js b/addon/resolver.js index 7f8c999..ac240b9 100644 --- a/addon/resolver.js +++ b/addon/resolver.js @@ -13,7 +13,7 @@ export default Resolver.extend({ // First try to resolve with React-styled file name (e.g. SayHi). // If nothing is found, try again with original convention via `resolveOther`. let result = - this.resolveReactStyleFile(parsedName) || this.resolveOther(parsedName); + this._resolveReactStyleFile(parsedName) || this.resolveOther(parsedName); // If there is no result found after all, return nothing if (!result) { @@ -37,14 +37,16 @@ export default Resolver.extend({ resolveReactComponent(parsedName) { parsedName.type = 'component'; const result = - this.resolveReactStyleFile(parsedName) || this.resolveOther(parsedName); + this._resolveReactStyleFile(parsedName) || this.resolveOther(parsedName); parsedName.type = 'react-component'; return result; }, // This resolver method attempt to find a file with React-style file name. // A React-style file name is in PascalCase. - resolveReactStyleFile(parsedName) { + // This is made a private method to prevent creation of "react-style-file:*" + // factory. + _resolveReactStyleFile(parsedName) { const originalName = parsedName.fullNameWithoutType; // Convert the compnent name while preserving namespaces