fix: attr matching for empty tags in tag:attribute patterns
#133
Conversation
michael-ciniawsky
changed the title
attr matching for empty tags in tag:attribute patterns
README.md
Outdated
| @@ -27,6 +27,20 @@ By default every local `<img src="image.png">` is required (`require('./image.pn | |||
|
|
|||
| You can specify which tag-attribute combination should be processed by this loader via the query parameter `attrs`. Pass an array or a space-separated list of `<tag>:<attribute>` combinations. (Default: `attrs=img:src`) | |||
|
|
|||
| If you use `<custom-elements>`, and lots of them make use of a `custom-src` attribute, you don't have to specify each combination `<tag>:<attribute>`: just specify an empty tag like `attrs=:custom-src` and it will match every element. | |||
There was a problem hiding this comment.
Necessary to document both ways ? (honestly 😛 ) :attr && tag:attr ?
There was a problem hiding this comment.
@michael-ciniawsky Hi! Sorry but I'm not sure that I'm understanding what should be changed here... please could you clarify? Thanks!
|
Could you also rebase against latest master ? Why is it even outdated 🙃 :) |
nuragic
force-pushed
the
nuragic-proposal-fix
branch
from
2cf23b7 to
abc08f6
Compare
index.js
Outdated
| var res = attributes.find(function(a) { | ||
| return item.indexOf(a) >= 0; | ||
| if (a.startsWith(':')) { |
There was a problem hiding this comment.
@michael-ciniawsky startsWith method has been introduced in ES6 so... not sure if it's ok or I should replace it with the corresponding ES5 version ...
There was a problem hiding this comment.
node >= v4 Support ? What is the alternative ? I'm working on #134 which is v1.0.0 with min node >= v4. It would normally be good to stay with node >= v0.12 in the current semver minor range, but node >= v0.12 is EOL and eventually the next release includes #134 anyways 😛 but not 💯 sure yet.
const starts = (value, str) => str.substr(0, value.length) === valueThere was a problem hiding this comment.
Well, just checked on node.green. Supported on Node >= v4 (the v0.12 requires the --harmony flag... 😅 No worries, in this case a.charAt(0) === ':' is exactly the same so changing it right now. 😉
There was a problem hiding this comment.
or str.charCodeAt(0) === ':' || str[0] === ':' yep 😛
nuragic
force-pushed
the
nuragic-proposal-fix
branch
from
abc08f6 to
69e84c7
Compare
Codecov Report
@@ Coverage Diff @@
## master #133 +/- ##
==========================================
+ Coverage 96.9% 96.93% +0.03%
==========================================
Files 2 2
Lines 97 98 +1
Branches 18 19 +1
==========================================
+ Hits 94 95 +1
Misses 3 3
Continue to review full report at Codecov.
|
michael-ciniawsky
added
pr_action: Merge ready
status: Approved
and removed
pr_action: Review
labels
|
Looks like you need to rebase :) What are the release schedules, anyway ? I'd love to have this in an official relase. |
|
We can schedule a release soon after this PR. |
|
@andersevenrud Uhm... weird... it was up to date... @hemanth did someone (Rebased BTW). |
Fix a bug introduced with webpack-contrib#129.
nuragic
force-pushed
the
nuragic-proposal-fix
branch
from
69e84c7 to
ea095e3
Compare
As reported in #129 (comment) there was a bug in the previous implementation, so I've just fixed it and added a test to cover that case.
masterbecause there's no other branch apparently. However, as also reported in the above comment, people is looking at docs inREADME.md(and eventually the code) but their local version is not yet aligned because the code inmasteris not released, so it would be nice to create adevbranch. 😌