Skip to content

Initial implementation of co-located templates RFC.#249

Merged
rwjblue merged 34 commits intomasterfrom
colocation
Sep 24, 2019
Merged

Initial implementation of co-located templates RFC.#249
rwjblue merged 34 commits intomasterfrom
colocation

Conversation

@rwjblue
Copy link
Member

@rwjblue rwjblue commented Jun 30, 2019

Still quite WIP, but "basically works" when using ember-cli 3.11 and Ember latest canary (including changes from emberjs/ember.js#18158.

Still quite WIP, but "basically works" when using ember-cli master which includes

ember-cli/ember-cli@84c449a

Co-authored-by: Robert Jackson <me@rwjblue.com>
Copy link

@devotox devotox left a comment

Choose a reason for hiding this comment

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

This breaks this addon https://github.com/sethwebster/ember-cli-new-version/tree/master/addon/components/new-version-notifier with this error

Error: Could not find module ember-cli-new-version/components/new-version-notifier/template imported from ember-cli-new-version/components/new-version-notifier/component

@chriskrycho
Copy link
Contributor

This is currently breaking with the @classic decorator invocation because of how it's transforming the component declaration.

SyntaxError: /Users/chris/dev/test/new-stuff-yo/new-stuff-yo/components/lame.js: Leading decorators must be attached to a class declaration (6:0)

  4 | 
  5 | @classic
> 6 | const CLASS = class Lame extends Component {
    | ^
  7 | }
  8 | 
  9 | const setComponentTemplate = Ember._setComponentTemplate;

This isn't a problem with the defaults, since the default behavior is to generate a .extend({ ... })-style class, but for anyone migrating via @classic and using colocation, it's a breaker, and one without a workaround. (For folks looking on: the transform as written actually matches the ES spec, but the Stage 1 transforms and any TS-dependent editor tooling, including VS Code's, falls down on the revised spec; and all existing docs I'm aware of specify the Stage 1 export position.)

Robert Jackson added 4 commits September 5, 2019 10:20
We can now rely on Node 8+, so this is "safe".
Running `BROCCOLI_DEBUG=ember-cli-htmlbars:* ember b` will now emit
files into `DEBUG/ember-cli-htmlbars/` showing the various stages of
compilation (input, after colocation processing, and final output).
{
"name": "ember-cli-htmlbars",
"version": "3.1.0",
"version": "3.2.0-alpha.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

We'll need to tweak this, as there are breaking changes now on master.

@rwjblue
Copy link
Member Author

rwjblue commented Sep 5, 2019

@chriskrycho - Can you share a snippet of what you started with? The error doesn't make it clear...

@chriskrycho
Copy link
Contributor

Yes, of course! Original input:

import Component from '@ember/component';
import classic from 'ember-classic-decorator';

@classic
export default class Lame extends Component {}

@pzuraq
Copy link

pzuraq commented Sep 5, 2019

Got an issue popping up over in ember-set-helper: adopted-ember-addons/ember-set-helper#6

Seems like a custom transform is not being run against colocated templates. I copied the template in the reproduction app into the app/templates/components directory and things started working, so it seems to be something with ember-cli and colocation specifically.

Robert Jackson added 4 commits September 8, 2019 21:43
@rwjblue
Copy link
Member Author

rwjblue commented Sep 14, 2019

OK, landed the changes from #280 into here and that should have addressed quite a few of the issues folks have reported:

  • @chriskrycho's comment RE: using decorators before the export
  • @pzuraq's comment about preprocessors not running properly (we now do exactly the same as hbs, and that works well with custom transforms)

@rwjblue rwjblue marked this pull request as ready for review September 15, 2019 17:35
@rwjblue rwjblue changed the title Initial spike of co-located templates RFC. Initial implementation of co-located templates RFC. Sep 23, 2019
@rwjblue rwjblue merged commit 6acc791 into master Sep 24, 2019
@rwjblue rwjblue deleted the colocation branch September 24, 2019 00:05
@rwjblue rwjblue restored the colocation branch September 24, 2019 00:05
@rwjblue rwjblue deleted the colocation branch August 10, 2020 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants