ember-cli-stylelint icon indicating copy to clipboard operation
ember-cli-stylelint copied to clipboard

Support in-repo addons and engines

Open amk221 opened this issue 7 years ago • 17 comments

With a config like:

stylelint: {
  syntax: 'scss',
  testFailingFiles: true,
  includePaths: [
    'lib/engine1/addon/styles',
    'lib/engine2/addon/styles',
    'lib/inrepoaddon1/app/styles',
    'lib/inrepoaddon2/app/styles'
  ]
}

I get this error

Error: Merge error: file addon.stylelint-test.js exists in tmp/broccoli_merge_trees-input_base_path-vQ7dMwvy.tmp/1 and tmp/broccoli_merge_trees-input_base_path-vQ7dMwvy.tmp/2
Pass option { overwrite: true } to mergeTrees in order to have the latter file win.
    at BroccoliMergeTrees._mergeRelativePath (node_modules/broccoli-merge-trees/index.js:278:17)
    at BroccoliMergeTrees.build (node_modules/broccoli-merge-trees/index.js:83:24)
    at node_modules/broccoli-plugin/read_compat.js:93:34
    at tryCatch (node_modules/rsvp/dist/rsvp.js:525:12)
    at invokeCallback (node_modules/rsvp/dist/rsvp.js:538:13)
    at publish (node_modules/rsvp/dist/rsvp.js:508:7)
    at flush (node_modules/rsvp/dist/rsvp.js:2415:5)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

amk221 avatar Sep 27 '17 15:09 amk221

ill take a look over the weekend to see how i can get it working for engines let you know

billybonks avatar Oct 25 '17 18:10 billybonks

I've run into the same issue. I have two in-repo engines and one in-repo addon that contains shared classes...

/lib
  /engine-one
  /engine-two
  /shared-classes

Without providing any configuration options, I noticed that the conditional check in lintTree passes twice, once for the main app and another for the in-repo-addon. This is why it's presenting the error that everyone is seeing. After making the following changes to index.js...

lintTree(type, true) {
  if (type === 'app' && tree.destDir === '/') {
    // unchanged code
    return mergeTrees(linted, { overwrite: true });
  }
}

And adding the following to the parent app's ember-cli-build.js file...

stylelint: {
  generateTests: true,
  includePaths: [
    'lib/engine-one/addon/styles',
    'lib/engine-two/addon/styles',
    'lib/shared-classes/app/styles'
  ]
}

...the build is now successful and all .scss files are linted correctly.

I don't know if this is the proper way to address this but it has fixed our build issues. Figured I'd post this solution as an potential option and open it up to feedback. I would be happy to submit a merge request as well.

bartsqueezy avatar Mar 19 '18 21:03 bartsqueezy

ye would like to have a pull request for this, and/pr you able to write a few unit tests as well/ make a demo app with this configuration working.

On Tue, Mar 20, 2018 at 5:16 AM Steve Bartnesky [email protected] wrote:

I've run into the same issue. I have two in-repo engines and one in-repo addon that contains shared classes...

/lib /engine-one /engine-two /shared-classes

Without providing any configuration options, I noticed that the conditional check in lintTree passes twice, once for the main app and another for the in-repo-addon. This is why it's presenting the error that everyone is seeing. After making the following changes to index.js...

lintTree(type, true) { if (type === 'app' && tree.destDir === '/') { // unchanged code return mergeTrees(linted, { overwrite: true }); } }

And adding the following to the parent app's ember-cli-build.js file...

stylelint: { generateTests: true, includePaths: [ 'lib/engine-one/addon/styles', 'lib/engine-two/addon/styles', 'lib/shared-classes/app/styles' ] }

...the build is now successful and all .scss files are linted correctly.

I don't know if this is the proper way to address this issue but it has fixed our build issues. Figured I'd post this solution as an potential option and open it up to feedback. I would be happy to submit a merge request as well.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/billybonks/ember-cli-stylelint/issues/73#issuecomment-374380789, or mute the thread https://github.com/notifications/unsubscribe-auth/ABkGyOJI5So5dZkjAAyExNbenhxCDFgDks5tgCAugaJpZM4Pl92r .

billybonks avatar Mar 20 '18 04:03 billybonks

Just to update everyone on this thread, making some pull requests to ember-cli that will make styles first class citizens for linting that should resolve all quirks but at the same time it means i will deprecate includePaths since that config is a workaround

https://github.com/ember-cli/ember-cli/pull/7713

billybonks avatar Mar 30 '18 12:03 billybonks

@bartsqueezy https://github.com/billybonks/ember-cli-stylelint/releases/tag/2.1.0

if you upgrade to this version your includedPaths should work now without changing the source please let me know :)

billybonks avatar Apr 04 '18 13:04 billybonks

@billybonks, thank you for spending the time to get this issue corrected. Much very appreciated!

I pulled v2.1.0 and I still get presented with an error message...

Merge error: file addon.stylelint-test.js exists 
Pass option { overwrite: true } to mergeTrees in order to have the latter file win.

I think the final remaining change is to add { overwrite: true } as the 2nd argument when calling mergeTrees. After making that change locally, builds were back up and running again.

bartsqueezy avatar Apr 04 '18 21:04 bartsqueezy

seems to be the same issue as https://github.com/billybonks/ember-cli-stylelint/issues/49, probably don't want to override cause then you loose tests, will investigate this next

billybonks avatar Apr 05 '18 05:04 billybonks

okay the easiest solution to this is to use this config

    stylelint: {
      includePaths: [
        'lib',
        ]
    }

billybonks avatar Apr 05 '18 15:04 billybonks

@billybonks, I have multiple engines and addons under /lib. Does it make a difference if you reference each directory directly instead of just the parent?

stylelint: {
  generateTests: true,
  includePaths: [
    'lib/engine01/addon/styles',
    'lib/engine02/addon/styles',
    'lib/engine03/addon/styles',
    'lib/addon/app/styles'
  ]
}

With this config, I still get the merge error during a build.

bartsqueezy avatar Apr 05 '18 17:04 bartsqueezy

right now if you use lib/engine01/addon/styles and you have file style.scss the output will be style.test.js, now if lib/engine02/addon/styles also has style.scss it will output the same file in the same virtual directory. so if you use the overwrite config you actually loose one of the tests.

but if you reference the parent lib then the above 2 files in the virtual directory will be lib/engine01/addon/styles/style.test.js lib/engine02/addon/styles/style.test.js

so they wont override. in theory i can move them after i generate the tests, but won't be able to work on that until 18april since i will be offline.

using the parent lib should be totally safe, the only thing i would look out for is if it increases your built times, because it broccoli will do a bit more processing but i think it shouldn't be very bad.

billybonks avatar Apr 05 '18 18:04 billybonks

@billybonks, I confirmed this working on my end. Specifying only the parent lib dir in the includePaths option showed a negligible difference in load times. Thank you again for getting a workaround merged! Hoping we get some traction in ember-cli repo for addressing this correctly 🤞

bartsqueezy avatar Apr 20 '18 17:04 bartsqueezy

This continues to be a problem. We are using the following addon combinations:

    "ember-cli-stylelint": "3.0.2",
    "ember-cli-template-lint": "2.0.0",
    "stylelint": "13.0.0",
    "stylelint-config-standard": "19.0.0",
    "stylelint-order": "4.0.0"

We have the following settings;

stylelint: {
  includePaths: ['addon']
},

We receive the below error:

Build Error (BroccoliMergeTrees)

[BroccoliMergeTrees] error while merging the following:
  1.  [SimpleConcatConcat]
  2.  [SimpleConcatConcat]
Merge error: file true.stylelint-test.js exists in C:\Users\[user]\AppData\Local\Temp\broccoli-37716Flv0TMg2N5eI\out-040-simple_concat_concat and C:\Users\[user]\AppData\Local\Temp\broccoli-37716Flv0TMg2N5eI\out-045-simple_concat_concat
Pass option { overwrite: true } to mergeTrees in order to have the latter file win.

We have multiple _style.scss files against multiple folders and components, as well as files that group them together in a global directory. Basically, we have styling per component. Our directory structure looks something like:

/addon/component/mycomponent1/_style.scss
/addon/component/mycomponent2/_style.scss
/addon/component/mycomponent3/childcomponent/_style.scss
/addon/css/bundle.scss

The workarounds listed above do not work for us.

Using version 2.2.0 of this addon allows everything to work.

deleemillie avatar Jan 16 '20 04:01 deleemillie

Howdy @billybonks! I ran into this today trying to upgrade from 2.2.0 to 4.0.0. I got the error in an internal addon (not an in-repo addon, just a regular addon that happens to be private to our company) that has scss files in both app/styles as well as tests/dummy/styles. The error was:

➜  yapp-ember-kit git:(chore/release-it) ✗ ember s
Build Error (BroccoliMergeTrees)

[BroccoliMergeTrees] error while merging the following:
  1.  [SimpleConcatConcat]
  2.  [SimpleConcatConcat]
Merge error: file true.stylelint-test.js exists in /var/folders/vs/8wls8rjn4gqd4m2ydxywp6v00000gn/T/broccoli-92843PN6bPkEhpOtY/out-039-simple_concat_concat and /var/folders/vs/8wls8rjn4gqd4m2ydxywp6v00000gn/T/broccoli-92843PN6bPkEhpOtY/out-043-simple_concat_concat
Pass option { overwrite: true } to mergeTrees in order to have the latter file win.


Stack Trace and Error Report: /var/folders/vs/8wls8rjn4gqd4m2ydxywp6v00000gn/T/error.dump.2d289c976fbaf6253f66e56347290ca4.log

lukemelia avatar May 17 '20 17:05 lukemelia

Hey, will take a look at this again on the weekend.

billybonks avatar May 18 '20 16:05 billybonks

@billybonks thanks. happy to remote pair on it if you want. you can ping me on discord.

lukemelia avatar May 18 '20 17:05 lukemelia

Hallo @billybonks! I just wanted to add to this thread and state that this error is still prevalent in version 4.0.0, and also to add a quick workaround for anyone who is running into this issue.

While working on adding ember-cli-stylelint in an addon, I received the following error:

Build Error (BroccoliMergeTrees)

[BroccoliMergeTrees] error while merging the following:
  1.  [SimpleConcatConcat]
  2.  [SimpleConcatConcat]
  3.  [SimpleConcatConcat]
Merge error: file true.stylelint-test.js exists in /var/folders/cq/gz2xyllj2714wz0wtd96x7700000gp/T/broccoli-65859uoOyr5SOMprb/out-023-simple_concat_concat and /var/folders/cq/gz2xyllj2714wz0wtd96x7700000gp/T/broccoli-65859uoOyr5SOMprb/out-028-simple_concat_concat
Pass option { overwrite: true } to mergeTrees in order to have the latter file win.

As a workaround, I went to ./node_modules/ember-cli-stylelint/index.js and modified the line:

return mergeTrees(linted);

to:

return mergeTrees(linted, {overwrite: true});

which ultimately fixed the error. Hope this helps others who might run into this issue!

auroraborghi avatar Jul 24 '20 18:07 auroraborghi

I've run into the same issue with an internal addon. I think the correct solution is to set group: false.

However this merge https://github.com/billybonks/ember-cli-stylelint/commit/33b63a832bce64487c759a584446319f0e4c3c41 seems to prevents anything but the default.

I assume the intention was to default only if undefined

this.styleLintOptions.group = this.styleLintOptions.group === undefined ? true : this.styleLintOptions.group;

vs always forcing true

this.styleLintOptions.group = true;

@billybonks can you confirm? Any chance this could be patched? 🙏

rrenno avatar Nov 10 '20 07:11 rrenno