Soluzione errore: TypeError non rilevato

Question

TypeError non rilevato: document.getElementById (...) è nullo

Ho un singolo file JavaScript collegato a più pagine. Lo snippet di seguito è una funzione utilizzata per una singola pagina. L' errore precedente viene visualizzato quando l'utente naviga in una delle pagine che non utilizza la funzione seguente e fa riferimento alla document.getElementById('button01').addEventListener('click', newThing);riga in fondo. Ho capito che l'errore si verifica perché button01non esiste in queste pagine.

function newThing() {
const output = document.getElementsByTagName('output')[0];
if (!(document.forms.thingSelection2.type.value in options)) {
    return false;
}
const list = options[document.forms.thingSelection2.type.value];
const method = document.forms.thingSelection1.mode.value + 'Item';
const item = list[method]();
output.innerHTML = item;

}
document.getElementById('button01').addEventListener('click', newThing);

Soluzione

La mia soluzione per questo è semplice; posiziona la riga in un'istruzione if in questo modo:

if(document.getElementById('button01')) {
    document.getElementById('button01').addEventListener('click', newThing);
}

Questo rimuove l'errore dalle pagine che non lo utilizzano.

Domande

Questo apre la possibilità a qualsiasi comportamento bug di cui non sono a conoscenza? C'è un modo migliore per ottenere gli stessi risultati?

HTML

Sono grato per il feedback ricevuto finora da questo post. Poiché alcune delle risposte contengono consigli basati su presupposti del mio HTML, ho deciso di aggiungere il rispettivo HTML nello snippet di seguito. Una cosa da notare è che uso due forme. Ciò è necessario per la funzionalità del codice. Non sono a conoscenza di eventuali bug non intenzionali che ciò potrebbe causare. In caso affermativo, fammelo sapere di seguito.

<div><output></output></div>
<div><button id="button01">New Thing</button></div>
<div>
   <form name="thingSelection1">
      <input type="radio" name="mode" value="random" id="mode1">&nbsp;Random
      <br/><input type="radio" name="mode" value="forward" id="mode2">&nbsp;Old&nbsp;-&nbsp;New
      <br/><input type="radio" name="mode" value="reverse" id="mode3">&nbsp;New&nbsp;-&nbsp;Old
   </form>
</div>
<div>
   <form name="thingSelection2">
       Doodle&nbsp;<input type="radio" name="type" value="doodle" id="doodleCheck"><br/>
       Video&nbsp;<input type="radio" name="type" value="video" id="videoCheck"><br/>
       Audio&nbsp;<input type="radio" name="type" value="audio" id="audioCheck"><br/>
       Photo&nbsp;<input type="radio" name="type" value="photo" id="photoCheck"><br/>
       Text&nbsp;<input type="radio" name="type" value="text" id="textCheck">
   </form>
 </div>

Inoltre, vorrei sottolineare che lo snippet originale è solo una parte del mio JavaScript completo. Sulla base delle informazioni fornite da questo post , ho concluso che l'aggiunta del rimanente JavaScript, sebbene possibilmente utile e pertinente per alcune delle risposte seguenti, potrebbe essere fuori dallo scopo dell'argomento originale di questo post. Si prega di notare i vostri pensieri su questo nei commenti qui sotto.

Answer 1

Questo apre la possibilità a qualsiasi comportamento bug di cui non sono a conoscenza?

Non è probabile. L' ifaffermazione va bene, anche se può essere resa più pulita:

• Piuttosto che selezionare l'elemento due volte (una volta per verificare se esiste, un'altra volta per richiamarlo addEventListener), salvalo in una variabile:

  const button = document.getElementById('button01');
  if (button) {
    button.addEventListener('click', newThing);
  }
  

• Oppure, se stai scrivendo JS che viene trasferito per la produzione (cosa che, in un progetto professionale o più grande, dovresti davvero essere), usa il concatenamento opzionale:

  document.getElementById('button01')?.addEventListener('click', newThing);
  

Ma rimane il problema fondamentale: il layout HTML è completamente scollegato da JavaScript. Dover verificare se esiste un pulsante in primo luogo è qualcosa di cui idealmente non dovrebbe essere un problema di cui preoccuparsi, almeno in un progetto più grande o più professionale. E se non avessi 1, ma 3 o 5 o 10 elementi con gestori su pagine diverse, che potrebbero o meno esistere? La base di codice sarebbe più difficile da mantenere di quanto dovrebbe essere.

Ci sono alcune soluzioni per questo:

• Un'opzione è avere un <script>file separato per le pagine con il modulo, ad esempio:

<form id="thingSelection2">
...
</form>
<script src="./thingSelection.js"></script>

dove thingSelection.jsaggiunge il listener di eventi. Ma ciò richiederà una richiesta separata al server, che potrebbe essere un problema su una pagina di grandi dimensioni su HTTP 1.1: se hai molti script diversi come questo, l'enorme numero di richieste parallele potrebbe rallentare le cose, specialmente per gli utenti con latenza elevata. (Il protocollo HTTP / 2 non ha problemi con connessioni aggiuntive allo stesso server IIRC)

(Puoi anche </form><script>// etc</script>inserire lo script, ad esempio, ma preferisco inserire gli script in file separati per consentire la memorizzazione nella cache)

• Ma se fossi in me, preferirei fortemente integrare completamente la creazione dell'HTML con i listener di eventi per quell'HTML, per renderlo completamente inconcepibile che uno esista senza l'altro, utilizzando un framework. Ad esempio, con React, potresti fare qualcosa sulla falsariga di:

const Things = () => {
    const [item, setItem] = useState('');
    const [selectedOption, setSelectedOption] = useState('foo');
    const clickHandler = (e) => {
        const fn = options[selectedOption];
        if (fn) {
          setItem(fn());
        }
    };
    return (
        <div>
            <select value={selectedOption} onChange={e => setSelectedOption(e.currentTarget.value)}>
                <option value="foo">foo</option>
                <option value="bar">bar</option>
            </select>
            <button onClick={clickHandler}>click</button>
            <output>{item}</output>
        </div>
    );
};

const options = {
  foo: () => 'foo',
  bar: () => 'bar',
};

const Things = () => {
    const [item, setItem] = React.useState('');
    const [selectedOption, setSelectedOption] = React.useState('foo');
    const clickHandler = (e) => {
        const fn = options[selectedOption];
        if (fn) {
          setItem(fn());
        }
    };
    return (
        <div>
            <select value={selectedOption} onChange={e => setSelectedOption(e.currentTarget.value)}>
                <option value="foo">foo</option>
                <option value="bar">bar</option>
            </select>
            <button onClick={clickHandler}>click</button>
            <output>{item}</output>
        </div>
    );
};
ReactDOM.render(<Things />, document.querySelector('.react'));

<script crossorigin src="https://unpkg.com/react@16/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@16/umd/react-dom.development.js"></script>
<div class="react"></div>

Quindi, quando sei su una pagina in cui sono necessarie le cose, esegui il rendering <Things />. Con questo approccio, l'HTML e i gestori JS associati sono tutti completamente autonomi nel file Things. Non è necessario utilizzare selettori che potrebbero avere collisioni involontarie. (Ad esempio, andando con il tuo codice originale di document.getElementById('button01'), e se qualche altra sezione completamente separata dell'HTML su una delle pagine avesse un elemento che utilizzava accidentalmente lo stesso ID? Allora avrai problemi.)

L'uso di un framework come questo è certamente molto da imparare, ma rende le basi di codice significativamente più gestibili. Ne vale la pena per progetti di medie o grandi dimensioni, IMO.

---

In una nota diversa, il tuo codice corrente potrebbe anche essere leggermente migliorato:

Preferisci selettori Le stringhe dei selettori sono generalmente più facilmente comprensibili e più flessibili rispetto ad altri metodi di selezione di elementi (come getElementsByTagNamee document.forms.someFormName.someFormElement). La stringa del selettore che corrisponde a un elemento si allineerà con il selettore CSS che definisce lo stile dell'elemento. Ad esempio, sostituirei:

document.forms.thingSelection2.type.value

con

document.querySelector('#thingSelection2 [name=type]').value

e

const output = document.getElementsByTagName('output')[0];

con

const output = document.querySelector('output');

(non è necessario selezionare una collezione quando è necessario solo il singolo elemento corrispondente)

Salva il valore Invece di selezionare ed estrarre il valore due volte, scrivi DRY code; mettilo in una variabile:

const { value } = document.querySelector('#thingSelection2 [name=type]');
if (value in options) { // Or, use `!options[value]
  return false;
}

Answer 2

Sembra che tu abbia incorporato alcuni dei consigli della mia risposta a uno dei tuoi altri post : è carino.

La risposta di CertainPerformance ha ottimi consigli. Seguendo l'essenza di esso, ci sono altre parti del codice che " potrebbero " essere problematiche:

const output = document.getElementsByTagName('output')[0];

Mentre dovrebbe esserci un <output>elemento se il codice HTML ne ha uno, se JavaScript viene eseguito prima che il DOM sia pronto, questo potrebbe portare a un errore proprio come farebbe il codice per ottenere un elemento dall'attributo id :

const output = document.getElementsByTagName('output')[0];
console.log('output tagName:', output.innerHTML)

Sarebbe meglio assicurarsi che document.getElementsByTagName('output')abbia una lunghezza diversa da zero prima di accedere al primo elemento.

---

Un altro suggerimento è di tornare il prima possibile - ad esempio, la riga da assegnare outputnon ha effetto sul condizionale:

const output = document.getElementsByTagName('output')[0];
if (!(document.forms.thingSelection2.type.value in options)) {
    return false;
}

Anche se probabilmente non fa differenza per questo codice se l'elemento di output viene recuperato dal DOM, la riduzione dei calcoli è una buona abitudine da sviluppare. In altre situazioni si potrebbe risparmiare tempo prezioso per gli utenti, ad esempio si prenda l'esempio di una richiesta lato server per recuperare i dati, che potrebbe richiedere alcuni secondi.

Answer 3

Presumo che il tuo HTML assomigli un po 'a questo:

<output></output>
<output></output>
<form>
  <input name="mode" value="..." />
  <input name="type" value="..." />
  <button>Click!</button>
</form>

Il mio consiglio principale non sarebbe quello di fare affidamento sugli ID poiché sono univoci per pagina. Considerare invece di fare affidamento sugli attributi dei dati e non presumere mai che ce ne siano un numero particolare. Il tuo javascript dovrebbe essere il più disconnesso possibile dal tuo layout HTML .

Modifichiamo un po 'quello che hai. Cominciamo con l'aggiunta di un attributo di dati che indica a JavaScript che hai un pulsante speciale:

<button data-my-widget>Click!</button>

Ciò consentirebbe di assegnare un listener di eventi a tutti i pulsanti con l'attributo dato e consentirebbe ai pulsanti di rimanere progressivamente migliorati:

const myThing = event => {
  ...
}

for (const button of document.querySelectorAll('[data-my-widget]')) {
  button.addEventListener('click', newThing);
}

Stai anche programmando a quale modulo si applica ogni pulsante - e i rispettivi controlli - all'interno del gestore di eventi. Puoi renderlo un po 'migliore usando l' attributo form su a buttone fornendo anche gli ID dell'elemento target come attributi dei dati:

<form>
  <input name="mode" value="...." />
  <input name="type" value="...." />
  <button data-my-widget data-mode="mode" data-value="type">Click!</button>
</form>

Quindi puoi accedere a questo modulo all'interno del gestore:

const myThing = event => {
  const outputs = document.getElementsByTagName('output');
  if (outputs.length === 0) {
    return;
  }

  const output = outputs[0];
  const target = event.target;
  const { form } = target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId } = target.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  if (mode === undefined || value === undefined || value in options === false) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }
  output.innerHTML = f();
}

Ciò mantiene il tuo javascript il più ignaro possibile della tua struttura HTML, anche se consiglierei comunque di sostituire anche la outputssezione, potenzialmente utilizzando attributi di dati, piuttosto che fare affidamento sull'indice dell'elemento:

<output id="output-1"></output> 
<button ... data-output="output-1">Click!</button>

const myThing = event => {
  ...
  const { mode: modeId, value: valueId, output: outputId } = target.dataset;
  ...
  const output = document.getElementById(outputId);
  output.innerHTML = ...;
}

Mettendo tutto insieme:

<output id="output-1"></output>
...
<output id="output-n"></output>
<form>
  <input name="mode" value="...." />
  <input name="type" value="...." />
  <button data-my-widget data-mode="mode" data-value="type" data-output="output-1">Click!</button>
</form>

const handleClick = event => {
  const { form } = event.target;
  if (form === undefined) {
    return;
  }

  const { mode: modeId, value: valueId, output: outputId } = target.dataset;
  const mode = form.elements[modeId]?.value;
  const value = form.elements[valueId]?.value;
  const output = document.getElementById(outputId);
  if (mode === undefined || value === undefined || output === undefined) {
    return;
  }

  const list = options[value];
  const f = list[`${mode}Item`];
  if (f === undefined) {
    return;
  }

  output.innerHTML = f();
}

for (const button of document.querySelectorAll('[data-my-widget]')) {
  button.addEventListener('click', handleClick);
}