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

Strict mode templates #496

Merged
merged 9 commits into from Apr 24, 2020
Merged

Strict mode templates #496

merged 9 commits into from Apr 24, 2020

Conversation

@chancancode
Copy link
Member

@chancancode chancancode commented Jun 2, 2019

Rendered

This has been in the works for a while (took a break from it while focusing on shipping Octane).

It is still a work in progress, but since I've been circulating it for a while, it probably makes sense to assign an official RFC number to it and get some initial feedback.

This RFC is intended to be complimentary to the template import RFC. Specifically, the intention is that:

  1. The main way for users to opt-into strict mode is via the template import syntax (which still has not be defined/finalized yet)

  2. Template imports are the main way to get things "into scope" in strict mode, since there are no implicit globals

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Jun 3, 2019

Thanks for making the PR for this @chancancode! I'd like to see migrating to attributes first (with an element modifier for setting properties) in the list of strict mode features also. Would you mind at least adding that to the list of unresolved questions?

@ef4
Copy link
Contributor

@ef4 ef4 commented Jul 4, 2019

Reading between the lines, this RFC implies to me that it will be legal to import a component into Javascript and then invoke the imported value in a template, so long as the template is in strict mode. It would be good to clarify that, because it's slightly different than only supporting invocation of values that were imported into templates.

One reason I care about the import-into-Javascript-first case is that it would compose with #507 to allow well-scoped "wildcards", both synchronous:

import { importSync } from '@ember/macros';
export default class extends Component {
  get FeedComponent() {
    return importSync(`./feed-components/${this.itemType}`).default;
  }
}
<this.FeedComponent />

and asynchronous:

import { restartableTask } from 'ember-concurrency-decorators';
export default class extends Component {
  init(...args) {
    super.init(...args);
    this.loadComponent.perform(this.itemType);
  }

  @restartableTask
  loadComponent = function*(itemType) {
    let module = yield import(`/feed-components/${itemType}`);
    this.set('FeedComponent', module.default);
  }
}
{{#if this.loadComponent.isRunning }}
  <LoadingSpinner />
{{else}}
  <this.FeedComponent />
{{/if}}

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 4, 2019

@chancancode - Can we add white space handling to the unanswered questions section? It seems plausible that a switch like this would enable us to also rethink how white space is handled at compile time (and we could strip white space by default, preserving only when explicitly stated)...

@Gaurav0
Copy link
Contributor

@Gaurav0 Gaurav0 commented Dec 18, 2019

One use case for dynamic component invocation is that sometimes we want our component library to be extendable, i.e. allow the developer using it to substitute their component for our default one. This is very common, e.g. ember-basic-dropdown, ember-power-select, ember-light-table, etc. Would enabling strict mode be possible for this scenario?

@chancancode and I had a conversation on Discord where he answered this:

https://discordapp.com/channels/480462759797063690/480462759797063692/656737872434561034

@ef4
Copy link
Contributor

@ef4 ef4 commented Dec 18, 2019

There are really two steps that can be present in the existing dynamic component invocation:

  1. Convert a string to a component definition.
  2. Invoke the component definition.

Today you can see these steps separately if you write:

{{component @custom}}

as

{{#with (component @custom) as |Custom|}}
  <Custom />
{{/with}}

In that form, it's only the (component @custom) part that isn't allowed in strict mode.

So addon authors who want to accept custom components still can, as long as the responsibility for importing them is moved to the callers.

Today, a "component definition" is different from a "component", and you can't just import a component and pass it as a value to be invoked. But that needs to change for strict mode to make sense. So users of your addon would ideally just Javascript import the component they want to pass to you, and pass it as a value, and you invoke it directly.

@gabrielcsapo
Copy link

@gabrielcsapo gabrielcsapo commented Apr 7, 2020

Since this changes the wiring format and that was a point of contention in #454 has that been resolved in this RFC? My thoughts on having the imports be resolvable in JS would allow tools to work as the JS community expects them. Right now for resolution to work for components in HBS would require changes in the template layer which I see as not being the area that needs improvement. Simply passing context from the backing component to the template seems like the most encapsulated way to move forward.

@pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Apr 7, 2020

@gabrielcsapo this RFC is primarily focussed on defining the behavior of strict mode templates and providing a low-level mechanism for creating them. The exact high-level format for how to do that is still being debated, and will be addressed in a separate, future RFC, after some time has passed with the primitives available for the community to experiment with.

@chancancode chancancode changed the title [WIP] Strict mode for templates Strict mode templates Apr 10, 2020

Note that this only affects arguments-less helpers, which are exceedingly rare,
as most helpers perform self-contained computations based on the provided
arguments. It also only affect argument positions. In content and attribute
Copy link
Member

@rwjblue rwjblue Apr 10, 2020

It also only affect argument positions.

This is vaguely concerning to me. I think it may be somewhat difficult to teach...

<MyComponent @value={{pi}} />
{{!-- passing the result of invoking the helper pi to the component --}}
<MyComponent @value={{(pi)}} />
Copy link
Contributor

@mydea mydea Apr 14, 2020

Just to be clear, this syntax is not supported yet, right? For me, it throws a parse error, e.g.:

<MyComponent @date={{(date-now)}} />

Copy link
Member Author

@chancancode chancancode Apr 14, 2020

Yes! We will have to make it work before we can deprecate

{{yield this.selectedComponent}}
```

This will make it clear to the human reader and enable tools to perform
Copy link
Contributor

@mydea mydea Apr 14, 2020

How will this work with template-only components? We have things like this quite often in our app, in template only components (simplified):

{{yield
  (hash Item=(component 'my-item') Header=(component 'my-header'))
}}

It would say it would be less than ideal to require us to provide a backing class for all of these components just to pass down all the yielded sub components, especially as we've just been working on making as many components template-only as possible.

Copy link
Contributor

@mydea mydea Apr 14, 2020

Or more specifically, I guess this will be solved with template imports at some point (probably) - but they aren't really mentioned in this RFC as far as I can tell, so I was wondering if

a) this would only ship after template imports
b) this would ship (potentially) before, but if you'd need this behavior you'd either need to wait on template imports to ship or create a backing JS class everywhere in the meanwhile
c) there would be an alternative way to achieve this - not that uncommon, I suspect - use case

I would of definitely prefer c) ;)

Copy link
Contributor

@pzuraq pzuraq Apr 14, 2020

Templates are compiled down to JavaScript in the end, so there already exist some artifacts that make them importable. Since the Component Template Co-Location RFC, this structure has been defined more thoroughly. Any template import preprocessor will be able to output something along the lines of:

export default setComponentTemplate(
  precompileTemplate('Hello, {{name}}!'),
  templateOnlyComponent()
);

And that would define a TO component which can be imported like any other component which has a backing class. The co-location RFC effectively already does this for existing TO components.

How template import preprocessors do this, exactly, is yet to be defined. The goal of this RFC is to provide a public primitive that template import preprocessors and syntax can be built on top of, not the actual import syntax itself. That will come in a future RFC.

Copy link
Member Author

@chancancode chancancode Apr 14, 2020

I think @mydea is asking how to do the contextual components pattern without introducing a JavaScript class. Using this hypothetical template import syntax:

---
import MyItem form '...';
import MyHeader from '...';
---

{{yield (hash Item=MyItem Header=MyHeader}}

For the example given in the RFC:

---
import First from '...';
import Second from '...';
import Third from '...';
---

{{#if (eq @selected "first")}}
  {{yield First}}
{{else if (eq @selected "second")}}
  {{yield Second}}
{{else if (eq @selected "third")}}
  {{yield Third}}
{{/if}}

...or...

---
import First from '...';
import Second from '...';
import Third from '...';
---

{{yield (get (hash first=First second=Second third=Third) @selected)}}

And for your second question, yes, this will ship first, but it is not really usable for end-users until template-import (or SFC) ships, which will be the primary intended way to "consume" strict mode. There are bigger problems than this pattern – you can't even invoke a regular component without importing it, so there isn't really a good way to use it without some surface syntax that works with import semantics. The plan is to land the primitive and being experimenting with these ideas, and eventually adopt one or more of them into mainline Ember.

Copy link

@sclaxton sclaxton Apr 17, 2020

Quick clarification: would you be able to pass the imported component to the component helper in order to curry arguments to the component yielded?

E.g.

---
import Item from '...';
---

{{yield (hash item=(component Item arg=something))}}

Copy link
Contributor

@pzuraq pzuraq Apr 17, 2020

I believe the answer is yes here, because I'm pretty sure this is already something that works with the {{component}} helper (would need to double check to be sure though)

Copy link
Member Author

@chancancode chancancode Apr 17, 2020

Yep. It doesn't work today yet, but it was added in the contextual helper + co-location RFCs. Or rather, the component helper has and will always be happy to take an invokable component as the first argument; the contextual helper + co-location RFCs stated the goal to make the component JS class itself invokable, which follows that the above will work.

@pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Apr 17, 2020

After discussing this in the meeting today, we are very excited to move this RFC into final comment period!

@knownasilya
Copy link
Contributor

@knownasilya knownasilya commented Apr 17, 2020

Yay!

@Gaurav0
Copy link
Contributor

@Gaurav0 Gaurav0 commented Apr 17, 2020

I am wondering if it will be possible to opt into strict mode per template or whether the primitive described (an option to precompile or compile) might force it to be an application wide setting. If the latter, we may box people in and make it too difficult to switch to strict mode in the future.

@pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Apr 18, 2020

@Gaurav0 the primitive defines templates on a per-component basis, meaning that you can have templates and components defined with strict-mode side-by-side with templates and components built the old fashioned way in sloppy mode.

The ability to convert incrementally is a very important goal, definitely agreed 😄 we've had this in mind and are making sure the design will enable this.

@Gaurav0
Copy link
Contributor

@Gaurav0 Gaurav0 commented Apr 18, 2020

"sloppy mode"? Can we find a term less pejorative? We don't need people thinking today's Ember is "sloppy".

@buschtoens
Copy link
Contributor

@buschtoens buschtoens commented Apr 18, 2020

@Gaurav0 "lax mode"?

@chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Apr 18, 2020

Perhaps the least contentious term is "classic"—one that I add has good "prior art" with Ember Classic.

Both "lax" and "loose" ("loose" being the most common use in cases like this) are, like "sloppy," somewhat accurate but not really the point—we don't want to have the classic mode forever, precisely because of the sloppy/lax/loose semantics it entails, but like a lot of the rest of Ember Classic that doesn't mean it was bad.

@NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Apr 18, 2020

Re "classic": at work, I've been having fun saying "Retro", because retro is like.. a cool hip word used with Sythwave / Retrowave Music. Not "bad", just reminiscent of the way things used to be.
image

@chancancode
Copy link
Member Author

@chancancode chancancode commented Apr 18, 2020

Pushed a commit to replace sloppy with non-strict for now. For what it's worth, the strict vs sloppy terminology came from JavaScript and I don't think it's particularly negative (and certainly does not express pejorative towards the programmer), as it is describing the strictness of the semantics. Just like JavaScript, I think it's useful to have a unique/unambiguous terminology for referencing it.

That being said, I am not attached to the particular naming, and I think even the word "strict" is not that useful in the long term – most of the differences here will be removed eventually (since they are already on the deprecation path for sloppy non-strict mode), so the main difference, at the end of the day, is just going to be how you get the variables into scope (imports vs magic globals). Hopefully, for most users, the strict vs sloppy non-strict distinction will never need to enter their lexicon at all as they will only encounter the last difference in real life and would just refer to them as template that uses imports (or SFC) vs not.

Anyway, I think strict vs non-strict is fine as an internal/intimate-API level terminology, and if everyone is happy enough with them I would like to refocus the reviews on the other aspects of the proposal.

@chriskrycho
Copy link
Contributor

@chriskrycho chriskrycho commented Apr 18, 2020

💯 Seems good. I don’t have any outstanding concerns with any of the language options, and I’m excited about strict mode. :shipit:

@locks
Copy link
Contributor

@locks locks commented Apr 24, 2020

I'd like to thank everyone that got involved in pushing this RFC forward :) We discussed it again at today's core framework meeting and decided to accept the RFC and move to the next step of the process. While the RFC is nearing it's one-year birthday, much work is still ahead of us to make this a reality for Ember apps, so keep an eye out for the tracking issue if you are interested in contributing!

@locks locks merged commit ba5ce1b into master Apr 24, 2020
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the strict-mode branch Apr 24, 2020
chancancode added a commit that referenced this issue Jan 7, 2021
This resolves a small confusion between the two RFCs.
chancancode added a commit that referenced this issue Jan 7, 2021
This resolves a small confusion between the two RFCs.
chancancode added a commit that referenced this issue Jan 7, 2021
This resolves a small confusion between the two RFCs.
rwjblue added a commit that referenced this issue Jan 8, 2021
Clarify and amend import paths in RFC #496 and #671
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet