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

fix: strip data-test-* attributes without explicit value from production build #88

Merged
merged 4 commits into from
Apr 7, 2017

Conversation

raido
Copy link
Contributor

@raido raido commented Mar 31, 2017

Fixes use case like:

{{my-component "param1" data-test-something}}

Real world example:

{{#link-to 'route' data-test-link}}Link{{/link-to}}

This would fail on production build by being completely broken link, because link-to actually would receive 'route' undefined as input. As second argument is meant to be "model" on link-to, then this causes major problems.

Sorry for the package.json showing up in diff, rebased went a bit into woods.

@marcoow
Copy link
Member

marcoow commented Mar 31, 2017

Hm, we have this already but apparently seems to break for (optional) positional params.

@marcoow marcoow requested review from marcoow and Turbo87 March 31, 2017 19:59
@raido
Copy link
Contributor Author

raido commented Mar 31, 2017

It is a different thing, it is about binding to the element without a value. But PR fixes removing attributes from production build where attributeBindings for data-test-* is not done.

@raido
Copy link
Contributor Author

raido commented Mar 31, 2017

If you want to replicate the issue, simply set strip: true on development build and create a link-to like shown above - you'll get broken links, because link-to receives undefined as second parameter, because it tries to lookup data-test- from the controller.

@Turbo87 Turbo87 force-pushed the positional-params-data-test-attrs branch 2 times, most recently from 6b7a645 to 99980dc Compare April 7, 2017 11:25
@Turbo87 Turbo87 force-pushed the positional-params-data-test-attrs branch from 99980dc to 260d156 Compare April 7, 2017 11:38
@Turbo87 Turbo87 merged commit cfb712c into mainmatter:master Apr 7, 2017
@Turbo87
Copy link
Collaborator

Turbo87 commented Apr 7, 2017

released as v0.3.1, thanks! 👍

@raido raido deleted the positional-params-data-test-attrs branch April 9, 2017 01:23
@Turbo87 Turbo87 added the bug label Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants