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

[CHORE] Move ember-test-selectors from deps to devDeps #713

Closed
wants to merge 1 commit into from

Conversation

bantic
Copy link
Contributor

@bantic bantic commented Jul 11, 2019

ember-test-selectors does not need to be a dependency.
It was moved from devDeps to deps in #537, which appears to have been unintentional.

As an aside, the data-test-* attrs that ember-table uses, e.g.

'data-test-ember-table': true,

appear to leak out to consuming apps and should likely be removed.

@mixonic
Copy link
Member

mixonic commented Jul 12, 2019

@bantic I think the idea might have been that [data-test- attributes are used in the page object of that PR which is in turn published for apps to use in test. Thus it was added as a dep so any consuming apps would strip the data-test-?

And additionally where data-test- is passed an an arg to components the author wanted those to be published as attributes?

I'm also unsure about if when data-test- selectors is installed in the app, but not in the addon's dependencies, if the stripping takes place on the addon/ folder. Often build-time addons only operate on an addon/ folder for the addon and the app/ tree for an app.

Anyway, I'm fine moving this to a dev dependency or not 😬 it is just a complex situation. Can you document in README what happens for an end-user in a) dev/test b) prod when they 1) don't use ember-test-selectors 2) do use ember-test-selectors?

@bantic bantic requested a review from twokul July 12, 2019 15:01
@bantic
Copy link
Contributor Author

bantic commented Jul 12, 2019

@mixonic Ah, you're right. I thought I had tested this and confirmed that the data-test-* always leak to consumer apps but I just tried again and I see that they are stripped when running the consumer app in production environment. I had previously tried the published 2.0.0-beta.7 (not stripped) but the latest master (specifically, I pinned a local ember app to ember-table@aa56375ffae0d39325d8a) does strip them.
To add to the confusion, the beta.7 is not even the latest tag on npm, 2.0.0-beta.4 is, so in the consumer app if you ember install ember-table you get that older beta version.

Anyway, I think the right step here is to not move test-selectors but do add some docs that the page object exists: #715

@bantic bantic closed this Jul 12, 2019
@bantic bantic deleted the bantic/move-test-selectors-dep branch July 12, 2019 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants