Js compatible resolving by ef4 · Pull Request #16 · ef4/rfcs

ef4 · 2018-12-06T13:50:20Z

No description provided.

rwjblue

Thanks for putting this together, I'm very excited about it!

Some general thoughts observations (didn't seem obvious where to put an inline comment for these):

Element modifiers should be mentioned here (we have already accepted the modifier manager RFC) and work exactly the same as helpers.
I think it should be made even more clear that all components/helpers/modifiers (except those in config/template-globals.js and those provided by Ember as built-ins) will need to be imported once inside the MU structure.
I strongly agree that supporting foo.js and foo.hbs at the same time is important, however I also very strongly dislike the pattern and do not think we should actually suggest using the same file path except extension. (This includes another example that you make RE: foo/index.js and foo/index.hbs). I'm happy with this effectively being a linting rule, but it definitely needs to be something.

rwjblue · 2018-12-06T14:37:53Z

text/0000-js-compatible-resolution.md

+
+    There is no magic that alters the exported value. When it's imported (whether in Javacript or HBS), the value you get is the actual class.
+
+3. When both `.js` and `.hbs` exist for a component, the build system unobtrusively associates the template with the default value exported by the Javascript. The template must be co-located with the Javascript, having the same filename except for changing the extension to `.hbs`.


I totally dig this in principle, but there are specific edge cases that we need to make sure are handled properly when we actually implement. For example, Ember.Component's today can have a different .layout property per instance. I definitely do not think this is a blocker at all, but likely means that we need to add a getLayout / getTemplate capability to the component manager interface. In Ember.Component this method would have to look at component.layout first then fall back to this loose/unobtrusive association.

I'd like to see the detailed design specifically call out the getLayout capability I mentioned above.

The template must be co-located with the Javascript, having the same filename except for changing the extension to .hbs.

This would expect something like:

src/ui/components/ async-button.js async-button.hbs title-bar.js title-bar.hbs

Do you not mean to support this?

src/ui/components/ async-button/ component.js template.hbs title-bar/ component.js template.hbs

rfcs/text/0000-js-compatible-resolution.md

Lines 191 to 194 in 2d4e4a4

1. `./path/to/button.hbs` is a template-only component.

2. `./path/to/button/index.hbs` is a template-only component.

3. `./path/to/button.js` exports a component class and `./path/to/button.hbs` is its template.

4. `./path/to/button/index.js` exports a component class and `./path/to/button/index.hbs` is its template.

Taking the above into account, it looks like it. This has implications for the MU resolver regarding the default types for collections, right?

rfcs/text/0000-js-compatible-resolution.md

Lines 271 to 277 in 2d4e4a4

## App structure implications

Most of the MU structure remains unchanged. Specifically, you still need things like `src/data/models`, `src/init`, `/src/services`, and `/src/ui/routes/` because all these things are still handled by the Collection Resolver.

But _within_ component collections (meaning `src/ui/components` plus any `-components` directories nested under routes) we don't need to impose any particular structure. You can nest components where they're used, and access them via relative imports.

Within component collections, there is no longer any need for special `-components` or `-utils` private collections. And the names `component.js` and `template.hbs` lose their special meaning. There are no special rules for looking up components that differ from looking up any other modules.

aha!

rwjblue · 2018-12-06T14:39:14Z

text/0000-js-compatible-resolution.md

+
+    ```js
+    // the previous example code would get rewritten to something like this:
+    import { registerTemplate } from '@ember/hypothetical-private-api';


FWIW, I do not think this can actually be private API. I totally agree that this is a tooling API and end users do not need to deal with it or understand it.

I'm suggesting we lint against its direct usage...

It would be really nice to a) expose this API to addon authors and b) keep it generic, so that it may be used for different file types as well.

This is of particular interest for addons like ember-css-modules that currently have to run an hbs transform that injects the file path of the template into the template helpers, so that they can be associated with the co-located .css files.

rwjblue · 2018-12-06T14:42:23Z

text/0000-js-compatible-resolution.md

+```hbs
+{{! my-component/index.hbs }}
+---
+import { customHelper } from '.';


I'm not sure this works reliably? IMHO, better to do ./index.

rwjblue · 2018-12-06T14:46:26Z

text/0000-js-compatible-resolution.md

+
+An addon authored in MU is relatively free to organize its components and helpers as it wishes within the `/src/ui/components` directory. Addon authors should provide a top-level entrypoint that re-exports public components & helpers so they're accessible without deep specifiers, and so it's clear what consitutes the addon's public API.
+
+The prior MU RFCs were vague on top-level entrypoints, but they were consistent in insisting that import specifiers should equal on-disk paths. In concordance with that, we clarify that the top-level entrypoint of an MU addon (one that contains a `/src` dir) is its true `index.js` at the addon's root. The ember-cli-specific code that often appears there instead can be placed elsewhere thanks to the `ember-addon.main` option in package.json. (This is a preexisting feature, not a new feature in this RFC.)


We should not use the addon's top level index.js by default (though I totally agree that it should "work", it just shouldn't be what we recommend). Instead we should specify main as src/index.js and ember-addon.main (as you suggest below).

Specifically, this is due to an issue in ESLint. It is not possible to target rules at index.js in the project root without also applying those rules to all index.js files throughout the entire project. Using src/index.js + "main": "src/index.js", avoids this issue and still nicely follows very very common ecosystem patterns.

rwjblue · 2018-12-06T14:50:05Z

text/0000-js-compatible-resolution.md

+First, let's delinate two separate layers of concern:
+
+ - **Core Primitives**: the underlying rules that govern how the system works. These need to be clear and robust, and preferrably standards-driven.
+ - **Best-practice Guidelines**: the defaults, nudges, and lints that we use to help Ember developers be productive and avoid every team wasting time bike-shedding their own ways of doing things.


This RFC doesn't really discuss best practices at all. It seems to mostly be talking about what should work (aka the core primitives) but not what we actually suggest folks to do.

rwjblue · 2018-12-06T14:54:30Z

text/0000-js-compatible-resolution.md

+
+It would probably be good to synthesize all the remaining-valid parts of the two earlier RFCs into this document, so there is one authoritative resource.
+
+This design leaves routes and their templates unchanged from the prior MU RFC. That seems to be an uncanny gap. They're still named `template.hbs`, which is maybe OK, but then if you want to nest more components under them you still need to reserve a place under `-components` to avoid ambiguity with child routes, and that means your imports in the route template need to say `import thing from "./-components/thing"` which seems bad.


IMHO, we cannot get away without changing routes in MU design. Being forced to teach two very different systems is just too much. IMHO there is no real reason a route can't use the same unobtrusive association setup that was described above for both its controller and template.

Either way, doubling the teaching angle (how do I name xyz template) seems like a show stopper here to me...

chadhietala · 2018-12-06T15:36:43Z

I'm a huge +1 on all of this. I agree with @rwjblue Element Modifiers should be mentioned here. I also think we do need to address {{component}} as a lookup and call mechanism. As I see it there is either at retcon here or we need to just introduce a new constructor or pattern for dealing with data driven layouts.

knownasilya · 2018-12-06T16:22:33Z

Overall I like the approach and feel like it'll be complete once some of the gaps mentioned by @rwjblue are filled in.

One of the main issues with the frontmatter that I can see is linting, since the frontmatter is JS, but the imports are not used in that "JS context". The linter would need to know JS and HBS. This is important for tooling integration, and might be a small issue, but would grow into a bigger issue if single-file components do follow this same pattern.

rlivsey · 2018-12-06T16:24:18Z

text/0000-js-compatible-resolution.md

+<Button />
+```
+
+The frontmatter section (delimited by `---`) uses Javascript syntax, but it is limited to only allow import statements.


If this was delimited with <script> ... </script> then we'd get syntax highlighting for free in editors, and we're opening the door for single-file components then it would make sense that pretty much any JS could go in there?

Downside being that it may be more confusing to have special treatment for the first script tag in the document…

Also would potentially open the door to questions around lexical scoping or imports if it was allowed elsewhere in the document too (as would be expected for any other tag):

<div> <script> import Button from './foo/button' </script> <Button /> </div> <div> <script> import Button from './bar/button' </script> <Button /> </div>

🤔

I would actually consider making it <script type="module">. Not sure if that would be a red herring/footgun? lol (not sure on the term)

And if we are going the <script> route, might as well do <template> too.

While <script> seems more approachable at first I think it does add trouble like mentioned. It's not full JS and I think we'd get dinged for that and I could see it being really hard to teach and limit what people are doing.

Making <script> do special imports would be a MAJOR breaking change to how Ember treats HBS too since currently popping in a script tag "just works" as if you threw in a script tag in innerHTML for an element. Not saying that's a good practice to have, but it is currently possible in Ember templates...

rwjblue · 2018-12-06T16:24:47Z

One of the main issues with the frontmatter that I can see is linting, since the frontmatter is JS, but the imports are not used in that "JS context". The linter would need to know JS and HBS.

FWIW, I was just thinking that we would either have a lint-frontmatter rule in ember-template-lint and it would run your projects eslint directly on the frontmatter all by itself (as if it were JS) OR we just write a (hopefully?) small parser to allow eslint to run against .hbs files (for frontmatter validation only)...

knownasilya · 2018-12-06T16:27:12Z

@rwjblue the issue is that the linter will not know that the imports are used.

I think a simple solution might be auto adding ignore 'not used' around those imports via build tooling, but thats not great for DX.

rtablada · 2018-12-06T17:46:39Z

An alternative to this could be using a components hash similar to Vue which allows explicit declaration and allows all locally scoped components to be "Just JavaScript" imports.

While I'm proposing this as a possible alternative I think that having imports in JS impact HBS gets confusing without single file components, but it does allow for compatibility and flexibility that is not allowed in this initial import spec.

rtablada · 2018-12-13T17:22:39Z

Something that I'm not sure about with the proposal is that there's two things that are RFC'd here:

Switching from ./component.js to ./index.js and same for templates
Template import semantics

While I think both are useful and I realize the time nature of trying to get an import solution for AngleBrackets and MU working I do worry that a single RFC with these two drastically different (and impactful) changes may be confusing and lead to rejection of both parts based on nit-picking of the other...

buschtoens · 2019-01-09T18:33:27Z

text/0000-js-compatible-resolution.md

+1. `./path/to/button.hbs` is a template-only component.
+2. `./path/to/button/index.hbs` is a template-only component.
+3. `./path/to/button.js` exports a component class and `./path/to/button.hbs` is its template.
+4. `./path/to/button/index.js` exports a component class and `./path/to/button/index.hbs` is its template.


In addition to https://github.com/ef4/rfcs/pull/16/files#r246491839, does this also mean that private local lookup will be deprecated, since any component can essentially import any other component?

buschtoens · 2019-01-09T18:38:37Z

text/0000-js-compatible-resolution.md

+
+The prior MU RFCs were vague on top-level entrypoints, but they were consistent in insisting that import specifiers should equal on-disk paths. In concordance with that, we clarify that the top-level entrypoint of an MU addon (one that contains a `/src` dir) is its true `index.js` at the addon's root. The ember-cli-specific code that often appears there instead can be placed elsewhere thanks to the `ember-addon.main` option in package.json. (This is a preexisting feature, not a new feature in this RFC.)
+
+For example, an addon containing one component might have `src/ui/components/the-component.js` and `src/ui/componetns/the-component.hbs`. These would technically be resolvable via `import TheComponent from "the-addon/src/ui/components/the-component"`, but that is an ugly deep-import and it's better for addon authors to provide clear reexports at the addon root:


Suggested change

For example, an addon containing one component might have `src/ui/components/the-component.js` and `src/ui/componetns/the-component.hbs`. These would technically be resolvable via `import TheComponent from "the-addon/src/ui/components/the-component"`, but that is an ugly deep-import and it's better for addon authors to provide clear reexports at the addon root:

For example, an addon containing one component might have `src/ui/components/the-component.js` and `src/ui/components/the-component.hbs`. These would technically be resolvable via `import TheComponent from "the-addon/src/ui/components/the-component"`, but that is an ugly deep-import and it's better for addon authors to provide clear reexports at the addon root:

ef4 added 5 commits December 5, 2018 13:46

draft

ae8005c

clarify

8b76e1a

updates

5c07dc9

update

7e950ab

updates

2d4e4a4

rwjblue reviewed Dec 6, 2018

View reviewed changes

rlivsey reviewed Dec 6, 2018

View reviewed changes

tomdale mentioned this pull request Jan 8, 2019

Nested components with <AngleBrackets /> emberjs/rfcs#428

Closed

buschtoens reviewed Jan 9, 2019

View reviewed changes


		There is no magic that alters the exported value. When it's imported (whether in Javacript or HBS), the value you get is the actual class.

		3. When both `.js` and `.hbs` exist for a component, the build system unobtrusively associates the template with the default value exported by the Javascript. The template must be co-located with the Javascript, having the same filename except for changing the extension to `.hbs`.

	1. `./path/to/button.hbs` is a template-only component.
	2. `./path/to/button/index.hbs` is a template-only component.
	3. `./path/to/button.js` exports a component class and `./path/to/button.hbs` is its template.
	4. `./path/to/button/index.js` exports a component class and `./path/to/button/index.hbs` is its template.

	## App structure implications

	Most of the MU structure remains unchanged. Specifically, you still need things like `src/data/models`, `src/init`, `/src/services`, and `/src/ui/routes/` because all these things are still handled by the Collection Resolver.

	But _within_ component collections (meaning `src/ui/components` plus any `-components` directories nested under routes) we don't need to impose any particular structure. You can nest components where they're used, and access them via relative imports.

	Within component collections, there is no longer any need for special `-components` or `-utils` private collections. And the names `component.js` and `template.hbs` lose their special meaning. There are no special rules for looking up components that differ from looking up any other modules.


		An addon authored in MU is relatively free to organize its components and helpers as it wishes within the `/src/ui/components` directory. Addon authors should provide a top-level entrypoint that re-exports public components & helpers so they're accessible without deep specifiers, and so it's clear what consitutes the addon's public API.

		The prior MU RFCs were vague on top-level entrypoints, but they were consistent in insisting that import specifiers should equal on-disk paths. In concordance with that, we clarify that the top-level entrypoint of an MU addon (one that contains a `/src` dir) is its true `index.js` at the addon's root. The ember-cli-specific code that often appears there instead can be placed elsewhere thanks to the `ember-addon.main` option in package.json. (This is a preexisting feature, not a new feature in this RFC.)


		It would probably be good to synthesize all the remaining-valid parts of the two earlier RFCs into this document, so there is one authoritative resource.

		This design leaves routes and their templates unchanged from the prior MU RFC. That seems to be an uncanny gap. They're still named `template.hbs`, which is maybe OK, but then if you want to nest more components under them you still need to reserve a place under `-components` to avoid ambiguity with child routes, and that means your imports in the route template need to say `import thing from "./-components/thing"` which seems bad. No newline at end of file


		The prior MU RFCs were vague on top-level entrypoints, but they were consistent in insisting that import specifiers should equal on-disk paths. In concordance with that, we clarify that the top-level entrypoint of an MU addon (one that contains a `/src` dir) is its true `index.js` at the addon's root. The ember-cli-specific code that often appears there instead can be placed elsewhere thanks to the `ember-addon.main` option in package.json. (This is a preexisting feature, not a new feature in this RFC.)

		For example, an addon containing one component might have `src/ui/components/the-component.js` and `src/ui/componetns/the-component.hbs`. These would technically be resolvable via `import TheComponent from "the-addon/src/ui/components/the-component"`, but that is an ugly deep-import and it's better for addon authors to provide clear reexports at the addon root:

Conversation

ef4 commented Dec 6, 2018

Uh oh!

rwjblue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chadhietala commented Dec 6, 2018

Uh oh!

knownasilya commented Dec 6, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knownasilya Dec 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rwjblue commented Dec 6, 2018

Uh oh!

knownasilya commented Dec 6, 2018

Uh oh!

rtablada commented Dec 6, 2018

Uh oh!

rtablada commented Dec 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

knownasilya Dec 6, 2018 •

edited

Loading