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

Add Babel 5 plugin stripping "data-test-*" properties #45

Merged
merged 4 commits into from
Jan 11, 2017

Conversation

Turbo87
Copy link
Collaborator

@Turbo87 Turbo87 commented Jan 11, 2017

This plugin will be added automatically to the Babel configuration of the host app and removes all data-test-* properties that it finds in JS code.

@Turbo87 Turbo87 requested review from marcoow and pangratz January 11, 2017 15:40
Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

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

awesome!

@pangratz
Copy link
Contributor

pangratz commented Jan 11, 2017

So what happens if I have a:

myProperty: computed(function() {
  let myCustomHash = {
    "data-test-ignored": "yes or no"
  };

  doSomethingWithMyCustomHash(myCustomHash);
})

Then this hash would be touched as well, right?

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Jan 11, 2017

@pangratz correct. I initially wrote it only for properties in .extend({ ... }), but then simplified it down. do you think your usage pattern is something that people are using and expecting not to be stripped?

@pangratz
Copy link
Contributor

I would say it is a use case which shouldn't occur that often really. But if I would have such a use case, I would expect it not to be stripped I guess 🤔 . This is though.

Only restricting to .extend({}) is too narrow, because then mixins would be excluded for example.

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Jan 11, 2017

FWIW you can still do this:

myProperty: computed(function() {
  let myCustomHash = {};
  myCustomHash["data-test-ignored"] = "yes or no";

  doSomethingWithMyCustomHash(myCustomHash);
})

@pangratz
Copy link
Contributor

Yeah, good point.

I guess this is not really an issue since the use case I posted would be a really special one.

@marcoow
Copy link
Member

marcoow commented Jan 11, 2017

@pangratz, @Turbo87: I think this is similar to the other thing we discussed - we can just go ahead and strip all properties that we find anywhere and then maybe narrow it down if people actually do run into problems.

@marcoow marcoow merged commit 9c0d0e3 into mainmatter:master Jan 11, 2017
@Turbo87 Turbo87 deleted the babel branch January 11, 2017 16:06
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.

3 participants