[Web Inspector] Port Web Inspector Style Guide from Trac

[burg]

The Web Inspector coding style guide lived on trac.webkit.org (now defunct) and was completely inaccessible to new contributors. Port and modernize it into docs/Deep Dive/Web Inspector/StyleGuide.md.

Covers:

• Formatting (indentation, strings, braces, trailing commas, semicolons)
• Variable declarations (let vs const -- WI uses let-by-default)
• Naming conventions (classes, properties, event handlers, localization)
• Class structure and member ordering with section comments
• Architecture (Manager/Model/View separation, protocol firewall)
• View lifecycle (initialLayout, layout, attached/detached)
• Event system (declaring, dispatching, listening, one-shot, await)
• Collections and iteration patterns
• Arrow function conventions
• Async patterns and promise gotchas
• Assertions
• CSS conventions (z-index, theming, class names)

The guide is structured with a Quick Reference of the most common review issues at the top, and detailed sections below. Rules were validated against the current codebase -- outdated Trac rules were dropped or updated.

Resolves #144

[the-chenergy]

This should be 6., and I don't quite understand the rule—this rule should be stated positively to be parallel to the other rules, like "Use this in addEventListener" if the rule is to not forget it. Although I don't get what this rule is saying; is it suggesting we always write this.addEventListener(event, handler) rather than addEventListener(event, handler)? In that case I think we shouldn't call the this. in the front one of the arguments

[the-chenergy]

Ah, I see that this rule refers to the Listening and Dispatching section later

[the-chenergy]

Suggested change

	Always use semicolons. No ASI reliance.
	Always use semicolons. No Automatic Semicolon Insertion reliance.

[the-chenergy]

I think given not all of these example code snippets include a file source, perhaps we should be consistent and just not include it here. The contents of the snippet and whether it's a good or bad example should be more important to focus on. Omitting the source also protects us against moving or editing that source later

Same applies to the snippet in the Trailing Commas section

[dcrousso]

awesome! 🤩

[dcrousso]

it's really only for methods and top-level functions

nested functions (including arrow functions) are fine to have on the same line such as

class Foo {
    bar()
    {
        function nested() {
            /* ... */
        }

        this.baz(() => {
            /* ... */
        });
    }
}

[dcrousso]

model objects are also allowed to call into the protocol

[dcrousso]

i think it's worth clarifying this is only for stuff related to WI.Object

i.e. EventTarget.prototype.addEventListener does not have a thisObject 3rd parameter

[dcrousso]

also multi-line array literals too

[dcrousso]

we do have a few classes that dont live in this namespace, but they're either super generic utilities (e.g. MultiMap, Debouncer, etc.) or only exist in Worker (e.g. HTMLParser, JSFormatter, etc.)

[dcrousso]

i think we should also say that super should always be invoked unless there's a really good reason not to in that case

[dcrousso]

i think we dont actually want to do this anymore as i dont think creating a Symbol is as cheap as just having a raw string literal

the primary benefit of Symbol is that the actual value is unique (e.g. Symbol("test") !== Symbol("test")) which is not really something we actually care about for these kinds of values

typically we only use Symbol as a way of creating a unique identifier in a loop (e.g. promiseIdentifier in Debouncer) or as an expando property on objects (e.g. WI.TabBrowser.NeedsResizeLayoutSymbol)

[dcrousso]

ditto above about clarifying that this is only for WI.Object

[dcrousso]

you also have to provide a thisObject to awaitEvent

[dcrousso]

generally speaking we usually only have single-line arrow functions if we're using the return value, so for something like this we've usually suggested multiple lines

target.DOMAgent.getDocument()
    .then((result) => {
        /* ... */
    })
    .catch((error) => {
        WI.reportInternalError(error);
    });