Unable to override addon re-exports in the consumer application

[ro0gr]

repo with reproduction. This reproduction is based on a dummy app but it's also reproducible within a host app.

Steps to reproduce:

$ ember g template test
$ ember g template test --dummy

produces following files:

• addon/templates/test.hbs
• app/templates/test.js
• tests/dummy/app/templates/test.hbs
• ...

Now if you run the build it will fail with an EEXIST exception:

ember build
...
EEXIST, file already exists '/home/beatle/workspace/ember-cli-apps/c/tmp/template_compiler-output_path-beowNp4d.tmp/dummy/templates/test.js'
Error: EEXIST, file already exists '/home/beatle/workspace/ember-cli-apps/c/tmp/template_compiler-output_path-beowNp4d.tmp/dummy/templates/test.js'
    at Error (native)
    at Object.fs.symlinkSync (fs.js:848:18)
    at symlink (/home/beatle/workspace/ember-cli-apps/c/node_modules/symlink-or-copy/index.js:82:14)
    at symlinkOrCopySync (/home/beatle/workspace/ember-cli-apps/c/node_modules/symlink-or-copy/index.js:58:5)
    at TemplateCompiler.Filter._handleFile (/home/beatle/workspace/ember-cli-apps/c/node_modules/broccoli-persistent-filter/index.js:101:12)
    at TemplateCompiler.<anonymous> (/home/beatle/workspace/ember-cli-apps/c/node_modules/broccoli-persistent-filter/index.js:88:21)
    at /home/beatle/workspace/ember-cli-apps/c/node_modules/promise-map-series/index.js:11:14
    at lib$rsvp$$internal$$tryCatch (/home/beatle/workspace/ember-cli-apps/c/node_modules/rsvp/dist/rsvp.js:493:16)
    at lib$rsvp$$internal$$invokeCallback (/home/beatle/workspace/ember-cli-apps/c/node_modules/rsvp/dist/rsvp.js:505:17)
    at lib$rsvp$$internal$$publish (/home/beatle/workspace/ember-cli-apps/c/node_modules/rsvp/dist/rsvp.js:476:11)

As I can see after some debugging it happens when broccoli tries to save compiled tests/dummy/app/templates/test.hbs to the tests/dummy/app/templates/test.js where the app/templates/test.js is merged already.

A little bit strange but this error happens only on cold starts and works ok on rebuilds.

[ro0gr]

I've checked if it's reproducible for another kind of files(coffeescript).
The answer is yes. It's the same as for .hbs.

So if you have the following addon:
some-addon/addon/components/test.coffee
some-addon/app/components/test.js

And the consumer application which tries to extend component from addon:
consumer-application/app/components/test.coffee
the build fails with the same EEXIST error.

[locks]

I'm also getting this for the application template in https://github.com/locks/ember-welcome-page.

[stefanpenner]

I believe this was addressed by: stefanpenner/fs-tree-diff@8a8cf15

[ro0gr]

Unfortunately no. I've updated reproduction app to use ember-cli@2.8.0 and have exactly the same error.
Also the latest fs-tree-diff is installed:

npm list fs-tree-diff
b@0.0.0 /home/ro0gr/workspace/tmp-apps/emcli-templates-collision-repro
└─┬ ember-cli@2.8.0
  └── fs-tree-diff@0.5.3 

@stefanpenner could you please reopen?

[ro0gr]

Maybe it's better to add a pr with a failing test here?
I can try to implement it.

[nathanhammond]

@ro0gr:

1. Can confirm that this does throw an error.
2. Inside of the dummy app I don't think we care; just rm tests/dummy/app/templates/test.hbs.

Does this throw an error in a consuming application which is separate from the dummy app?

The mental model here is that an addon is "pushing" something into the app's namespace. It's reasonable for that to error upon collision. I personally prefer that consumers of an addon handle this sort of thing via explicit imports from the addon's namespace which neatly sidesteps the problem and is more similar to our proposed future state.

I'm leaving this closed for now, but in the mean time we will still continue to receive notifications from any followup comments on this issue; being closed doesn't mean that we're ignoring the thread. (Stef closed ~30 on me in one day, so it's taking a while to get through the backlog.)

[ro0gr]

@nathanhammond thanks for your response.
I've got a chance to look at this issue under slightly different angle. Earlier I was under impression that host app sources applied to the tree after all addons sources are merged.

The mental model here is that an addon is "pushing" something into the app's namespace.

now I can see it works in an opposite order.

Does this throw an error in a consuming application which is separate from the dummy app?

Yes. #5336 (comment)
I've used a dummy app example in order to reduce use case(at least I've tried 😅 ).

This issue still makes me think that something is wrong here.
When I build this:

host-app/app/templates/my-template.hbs
charitable-addon/app/templates/my-template.hbs

build finishes successfuly and host-app template wins.

Then when I build this:

host-app/app/templates/my-template.hbs
charitable-addon/addom/templates/my-template.hbs
charitable-addon/app/templates/my-template.js

build fails with an EEXIST exception.

From my point of view here should not be a difference for a build result between these two use cases.

[ro0gr]

updated issue description. Mostly dropped irrelevant blocks of text and mentioned that issue is reproducible for separate app + adddon(not only for dummy app).

[leifdejong]

Still having this issue. Is it not possible to create a template in the addon and the exact template in the consumer app and have them work?

[ro0gr]

I've believed for a moment that #6480 might solve this issue. Unfortunately it doesn't.
Still can't figure out what's exactly is wrong here anyway I have some more details and leave it here.

Trying to debug this I've wrapped https://github.com/stefanpenner/broccoli-persistent-filter/blob/master/index.js#L146 with a try/catch statement:

  try {                                                                                                                             
    return this._handleFile(relativePath, srcDir, destDir, entry, outputFilePath, false, instrumentation);                            
  } catch (e) {                                                                                                                                                                                                                                                             
    console.log("patches", patches);                                                                                                
    throw e;                                                                                                                        
  }         

At the moment when the error happens:

Error: EEXIST: file already exists, symlink '/home/ro0gr/workspace/tmp-apps/emcli-templates-collision-repro/app/templates/test.js' -> '/home/ro0
gr/workspace/tmp-apps/emcli-templates-collision-repro/tmp/template_compiler-output_path-VGoDY8CT.tmp/dummy/templates/test.js'

We have the following patches:

[
    // create dir, .gitkeep...,
    ...,
  // host app template. 
  // comes from @host-app/app/templates/test.hbs
  [ 'create', 'dummy/templates/test.hbs', Entry {
      relativePath: 'dummy/templates/test.hbs',
      basePath: '/home/ro0gr/workspace/tmp-apps/emcli-templates-collision-repro/tmp/template_compiler-input_base_path-iFLYjtTe.tmp/0',
      mode: 33204,
      size: 23,
      mtime: 1475001132267 } 
  ],
  // template re-export from addon 
  // comes from @some-addon/app/templates/test.js
  [ 'create', 'dummy/templates/test.js', Entry {
      relativePath: 'dummy/templates/test.js',
      basePath: '/home/ro0gr/workspace/tmp-apps/emcli-templates-collision-repro/tmp/template_compiler-input_base_path-iFLYjtTe.tmp/0',
      mode: 33204,
      size: 44,
      mtime: 1480446201760 
  }]
]

According to patches both of these nodes should be created. But while applying patches they implicitly have the same output path after processing => dummy/templates/test.js, which causes an issue.

update: updated descriptions for patch entries

[stefanpenner]

Ah, that debugging i believe makes me understand the issue.

basically we have two files

dummy/templates/test.js // from the addons app directory
dummy/templates/test.hbs // from the app directory

Which really represent the same thing, but during the app + addon's app merge they did not collide, as hbs and js extensions made the files appear different.

So when the apps template compiler performed test.hbs -> test.js it did not expect test.js to already be present. I suspect broccoli-filter and broccoli-persistent-filter actually want to overwrite the target but broccoli-persistent-filter does not.

I believe the fix is https://github.com/stefanpenner/broccoli-persistent-filter/blob/master/index.js#L169 should try/catch detect the deoptimization scenario of error.name === 'EEXIST' and if so, unlink and try to symlink again.

quick braindump: broccolijs/broccoli-persistent-filter#88 (comment) actually i just also realized this is much harder, as we may need to resurrect the old file if the shadower is removed...

[ro0gr]

So when the apps template compiler performed test.hbs -> test.js it did not expect test.js to already be present.

As you can see from patches dumped above template re-export from addon tries to override .hbs template from the host.

When the error happens:

Error: EEXIST: file already exists, symlink '/home/ro0gr/workspace/tmp-apps/emcli-templates-collision-repro/app/templates/test.js' -> '/home/ro0
gr/workspace/tmp-apps/emcli-templates-collision-repro/tmp/template_compiler-output_path-VGoDY8CT.tmp/dummy/templates/test.js'

/tmp/template_compiler-output_path-VGoDY8CT.tmp/dummy/templates/test.js actually contains compiled handlebars template from the host app. Then the next patch(which contains template re-export from addon) tries to by applied and the error happens.

@stefanpenner this means if we try to unlink like you propose we just allow addon re-export override application template.

I feel the current exception is a right behavior. the invalid part is what we do have in a patches collection.

[stefanpenner]

As you can see from patches dumped above template re-export from addon tries to override .hbs template from the host.

Ah that then seems like a second issue. Will try and investigate this weekend.

[ro0gr]

yes, but that's what actually causes the issue.

looks like while @host-app/app/templates/ and @addon/app/templates trees are merging, both hbs and js files are merged to the same destination despite their output file extensions after processing.

[stefanpenner]

@ro0gr ya, that actually make this tricky. hmm..

[jamesarosen]

I'm working around this for now by doing

// tests/dummy/app/templates/components/freestyle-palette-item.js
import hbs from 'htmlbars-inline-precompile';
export default hbs`
<div>my template</div>
`;

[bichotll]

A simple workaround for that problem:

Target: [addon-name]/components/foo.hbs

1 - We create a new folder and a component with same name [our-app]/components/[addon-name]/foo.js.

2 - Create a template regarding that previous path [our-app]/templates/components/[addon-name]/foo.hbs and we modify whatever we need.

3 - We extend the component and we assign the new template:

// [our-app]/components/[addon-name]/foo.js

import Foo from '[addon-name]/components/foo.js';

// Workaround: It's not possible to override the template straight away using same path
// https://github.com/ember-cli/ember-cli/issues/5336
export default Foo.extend({
  templateName: 'components/[addon-name]/foo.hbs',
});

4 - Call {{[addon-name/foo]}} on your template and whalla!

[sdhull]

Note: templateName is no longer available. There is layoutName however it's part of private APIs and you shouldn't use it.

But you can import layout from 'my-app/templates/components/[addon-name]/foo'; and use layout: layout directly.