jQueryをもっとスマートにかけないでしょうか。

Question

ナビゲーションに子メニューを設定すると付与される「.item-has-children」というクラスが付いている場合に、親のメニューのaタグをdivにしてます。

基本的な事がよく分かっておらず、下記コードをもう少しスマートに書けないか（若しくは短縮）ご教授いただけませんでしょうか。

(function($) {
  if($('#gNavi').children().hasClass('menu-item-has-children')){
    var treeMenu1 = $('#item123.item-has-children > a');
    var treeMenu2 = $('#item002.item-has-children > a');
    $(treeMenu1).replaceWith(function() {
    	$(this).replaceWith('<div>'+$(this).html()+'</div>');	
    })
    $(treeMenu2).replaceWith(function() {
    	$(this).replaceWith('<div>'+$(this).html()+'</div>');	
    })
  }
})(jQuery);

追記
「#item123」と「#item002」のIDを持つ2つのメニューのみが対象です。

Answer 1

こんなところでしょうか。

• Edit fiddle - JSFiddle

<ul id="gNavi">
  <li id="item002" class="menu-item-has-children"><a href="#product002">item002</a></li>
  <li id="item123" class="menu-item-has-children"><a href="#product123">item123</a></li>
</ul>
<script>
'use strict';
(function ($) {
  if ($('#gNavi>.menu-item-has-children').length) {
    $('#item123>a,#item002>a').replaceWith(function () {  // id 属性値は一意の為、.item-has-children の条件を削除
      return $('<div></div>').append(this.childNodes);  // .html() で書き換えると addEventListener を代表とするDOM参照する機能が全て解除される(新しく参照が作られる為)ので既存のDOMノードを append() するように
    });
  }
}(jQuery));
</script>

(2015/12/16 12:31追記) wrapInner() では期待通りには動作しないという指摘を頂き、wrap() に修正しました。
(2015/12/16 14:57追記) 更に間違っていたので、replaceWith() に修正しました。

Comments

[hyper-drums-ko]

"wrapInner" だと '<a>...</a>' が '<a><div>...</div></a>' になるのではないでしょうか？

[think49]

仰る通りですね…。wrap() に修正しました。

[hyper-drums-ko]

今度は '<a>...</a>' が '<div><a>...</a></div>' になるのではないでしょうか？

'<a>...</a>' を '<div>...</div>' にすると思ったのですが。。。

[think49]

度々ありがとうございます。
どうやら、私が replaceWith() を機能を認識違いしていたようです。修正しました。

[cotton88]

ご回答頂き誠にありがとうございます。以前私も間違ってwrapInnerやwrapを使用してしまってhyper-drums-ko様のご指摘の通りの状況になってしまったので親近感が湧きました笑
ですがとても短くまとめていただいたので他のご回答者達と甲乙つけがたいのが悩みどころです。。

Answer 2

面白そうなので参加します。
HTML が見えないのでいろいろエスパーしていますが、これで行けるのではないかと。

$("#item123, #item002")
  .filter(".item-has-children")
  .find("a")
  .wrapInner("<div/>")
  .children("div")
  .unwrap();

よくわかっていないとのことで、(function($){ }(jQuery));も消しています。
もしどこかからコピペして紛れ込んでしまっただけなら、本来不要なコードです。
(実際に必要で書かれていたらすみません。)
これはjQueryの短縮名$がグローバル変数で使えないときにのみ有効なもので、詳しくはこちらを。

Comments

[think49]

> 本来不要なコードです。
"$" はいつ誰に使われてもおかしくない短い名前なので、前方互換性的には関数スコープに閉じ込める措置は意味のあるコードだと思います。
今現在、jQuery() しか著名なライブラリが $ を使っていないとしても、今後ずっと使われないという保証はありませんから、将来的に新しいライブラリを追加した時にコンフリクトするリスクを回避できます。
(個人的には jQuery() に統一して書くスタイルが分かりやすくて好きですが、流儀の違いだと思われるので私の回答では元の通りにしました。)

[tozjp]

> think49 さん
それほど高くないリスクなら、「スマートに、短縮」という意向を優先するのもありかと思います。$ を使う著名なライブラリでは prototype.js がありますが、むしろ経験上 jQuery のバージョン違い同士の競合のほうがある気がします。
多分そちらはこの方法では簡単に回避できませんね。

[think49]

> それほど高くないリスクなら、「スマートに、短縮」という意向を優先するのもありかと思います。
そういう考え方もありますが、リスクがなくなっているわけではないので「本来不要なコード」と断定するには弱い理由だと思います。
あと、コードを書く場所にもよりますが、即時関数を使わなくても下記コードで事足りる場合が多いですね。
jQuery(function ($) { /* この中では $ を使用できる */ });

> むしろ経験上 jQuery のバージョン違い同士の競合のほうがある気がします。
jQueryのバージョン違いは読み込み順に注意すれば回避可能な問題ではないでしょうか。

<script src="jquery-1.11.3.js"></script>
<script>/* jquery-1.11.3.js 用のコード */</script>
<script src="jquery-2.1.4.js"></script>
<script>/* jquery-2.1.4.js 用のコード */</script>

競合を回避する必要がある次のパターンです。

<script src="jquery-1.11.3.js"></script>
<script>jQuery1 = jQuery; /* jquery-1.11.3.js のオブジェクトを別のグローバル変数に退避する */</script>
<script src="jquery-2.1.4.js"></script>
<script>
jQuery1(function ($) { /* jquery-1.11.3.js 用のコード */ });
jQuery(function ($) { /* jquery-2.1.4.js 用のコード */ });
</script>

全てのjQueryコードが最新版で動くようにコードを書き換えるのが正攻法だと思いますが、コードを書き換えるコストを減らす事が目的なら多少の複雑化は許容しなければならないのかもしれません。
が、それにしてもグローバル名前空間を汚してまで上記方法を選択する理由が私には思いつきませんでした。

[tozjp]

理論上リスクが0でないことは理解しますが、jQueryに競合するライブラリが今後台頭するというのは現実的でしょうか。
それに、ちょっと精神論的になりますが、jQueryに競合させるような作者のライブラリがそれほど有用な出来栄えとも思えません。
すでに $ で市民権を得ているものがあるなら後発が譲るのが筋でしょうし、jQueryは先発の prototype との競合を回避する方法（jQuery.noConflict）を正式に用意しました。
実際は順番に気をつける必要はありません。

コスト減のため手間がかかる方法を当面可能性が低い状況に充てることも可能です。
qs = (function(){
var jq = jQuery.noConflict();
var qs = $;
$ = jq;
return qs;
})();

[think49]

> jQueryに競合するライブラリが今後台頭するというのは現実的でしょうか。
「prototype.js -> jQuery.js」「Underscore.js -> lodash.js」のように名前が競合するライブラリは過去にもリリースされた前例があります。

物事にはリスクがあり、リスクがより低い方法を選択するのは安全性の高いプログラミングをする上での基本だと思います。
jQuery, $ を比較した場合、$ の方がコンフリクトのリスクが高い為、少なくともグローバルスコープでは jQuery を使おう、という結論になります。

> それに、ちょっと神論的になりますが、jQueryに競合させるような作者のライブラリがそれほど有用な出来栄えとも思えません。
jQuery は prototype.js と競合させる変数名をつけても普及しました。
精神論の話をするのであれば、人間が読みやすいとはいえない記号のみの変数名($, _)を命名すべきではないと思います。
「Underscore.js, lodash.js, prototype.js, jQuery は既に認知されているから問題ない」というのは結果論に過ぎません。
本当に良い名前というのはその変数名を始めて見た人が中身を想像できる名前です(jQuery を知らない人が変数名を見ても中身を想像できなければなりません)。
そういう意味では $ という名前は使うべきではなく、jQuery のみを使うべきだと思います。
ES3 まで $ は機械的に生成する名前としてのみ許可されていましたが、ES5 でこの規約は削除されました。
この仕様変更は時代の流れを汲んだものだと思いますが、良い名前の変数名を命名する上では間違った選択だと思います。
現実的な問題としては $ や _ は grep で検索しづらいという問題もあります。変数名は適度な長さでユニークである方が有用です。

[cotton88]

tozjp様 ご回答誠にありがとうございます。面白そうといって頂きなんだか嬉しいです。
think49様 ご回答だけでなく御意見いただき誠にありがとうございます。

どちらが正しいのか分かり兼ねてしまいますがどちら様のコメントもとても勉強になって（と言っても話の7割くらいしかわかりませんが・・）分からないなりに調べてみようと思いました。本当にありがとうございます。

コードもこんな書き方があるのか！と感嘆の声がでました。今度ともお世話になるかと思いますがこの際はよろしくお願いいたします！

Answer 3

(function($) {
  if ($('#gNavi').children().hasClass('menu-item-has-children')) {
    $('#item123.item-has-children,#item002.item-has-children').children('a').each(function() {
      $(this).replaceWith('<div>' + $(this).html() + '</div>');
    });
  }
})(jQuery);

Comments

[cotton88]

ご回答誠にありがとうございます。他の回答者様たちと比べてとても短くて分かりやすいコードだと感じました。ご回答頂き大変助かりました！今度ともよろしくお願いいたします！

Answer 4

こんにちは。

一部ファンクションにしてみましたが、こんな感じでいかがでしょうか？
動けばいいのですが・・・

(function($) {
  function replace(elm){
    $(elm + ".item-has-children > a").replaceWith(function() {
        $(this).replaceWith('<div>'+$(this).html()+'</div>');    
    });
  }

  if($('#gNavi').children().hasClass('menu-item-has-children')){
    replace('#item123');
    replace('#item002');
  }
})(jQuery);

Comments

[cotton88]

質問させていただいてから早速のご回答ありがとうございます。もちろん動きました！
replace('#item002');の部分を増減させてやればとても見やすいですね。誠にありがとうございました！