1
\$\begingroup\$

Please, correct me with anything that I say in here (The actual question is below the code).

I've been quickly prototyping a project (ASP.NET Core 2.1) and haven't found the need to structure it with Webpack/Parcel/Rollup/Browserify as it's done with simple JavaScript validations per views (I do minify the files with Gulp).

Since I'm not enclosing my files into modules, they're all open to the global scope (bad idea, right?). Therefore, I've decided to "namespace" them by running everything inside a JavaScript variable.

The code that I have looks like this:

const formValidation = {
  submitBtn : null as unknown as HTMLButtonElement,
  errors: null as unknown as string[],
  errorsDiv: null as unknown as HTMLDivElement,
  gradeSections: null as unknown as HTMLDivElement,
  form: null as unknown as HTMLFormElement,
  mapDOM() {
    this.submitBtn = document.getElementById('js-btn-submit') as HTMLButtonElement;
    this.errorsDiv = document.getElementById('js-errors') as HTMLDivElement;
    this.gradeSections = document.getElementById('js-grade-sections')  as HTMLDivElement;
    this.form = document.getElementById('js-submit-form') as HTMLFormElement;
    this.mapEventListenersFormValidation();
  },
  mapEventListenersFormValidation() {
    this.submitBtn.addEventListener('click', this.submit.bind(this));
  },
  submit(evt: Event) {
    console.log(evt);
    evt.preventDefault();
    this.resetForms();
    if (!this.checkIfSubmitIsValid()) {
      this.showErrors();
      return;
    }
    this.form.submit();
  },
  checkIfSubmitIsValid() {
    this.errors = [];
    const subjectSelected = this.isSubjectSelected();
    const sectionsSelected = this.areSectionsSelected();

    if (!subjectSelected) {
      selectSubjectDropdown.classList.add('is-invalid');
      this.errors.push('Debe de seleccionar por lo menos una materia');
    }

    if (!sectionsSelected) {
      this.gradeSections.classList.add('form-error');
      this.errors.push('Debe de seleccionar al menos una sección');
    }

    return subjectSelected && sectionsSelected;
  },
  isSubjectSelected() {
    return selectSubjectDropdown.selectedIndex !== 0;
  },
  areSectionsSelected() {
    return selectedSectionCounter > 0;
  },
  showErrors() {

    if (!this.errorsDiv || !this.errorsDiv.parentElement) {
      return;
    }
    this.errorsDiv.parentElement.removeAttribute('hidden');
    this.errorsDiv.innerHTML = '';
    this.errors.forEach((error) => {
      const li = document.createElement('li');
      li.innerHTML = error;
      this.errorsDiv.appendChild(li);
    });
  },
  resetForms() {
    selectSubjectDropdown.classList.remove('is-invalid');
    this.gradeSections.classList.remove('form-error');
  },

};

DOMReady(formValidation.mapDOM.bind(formValidation));

Check the submitBtn, errors, and errorsDiv properties. AFAIK, JavaScript doesn't have a way to declare an "empty" property. Therefore something sane would be to assign it either undefined or null.

As I want to have it statically defined (through TypeScript), I want to assign a type to those properties. The problem is that I just can't assign a non-nullable type to a null.

As I'm trying to avoid non-implicit any's in the scripts, the compiler suggested me to use unknown.

The question is, whether it's ok for me to use the unknown type from TypeScript to cast a null to an unknown so it can let me cast it to the type that I'm going after.

Edit Ended up adding the whole document into it. I was working with that function, so it's different now.

\$\endgroup\$
  • \$\begingroup\$ The code to be reviewed looks sketchy or incomplete: where are checkIfSubmitIsValid() and showErrors() defined? \$\endgroup\$ – 200_success Nov 1 '18 at 14:14
  • \$\begingroup\$ @200_success: Oh they're not needed for what I'm looking for. It's the second and third line of the code snippet above. \$\endgroup\$ – Jose A Nov 1 '18 at 23:44
  • \$\begingroup\$ Maybe they're not needed for what you are looking for, but context is still important for reviewers. \$\endgroup\$ – 200_success Nov 1 '18 at 23:55
  • \$\begingroup\$ @200_success: Got it! I'll update it asap! Sorry \$\endgroup\$ – Jose A Nov 2 '18 at 0:07
0
\$\begingroup\$

The compiler can't know beforehand if document.getElementById('js-btn-submit') wil return anything. That is why you cannot assign the result directly to a type.

One solution is to tell the compiler that you are certain that the HTML element exists, using !

let button = document.getElementById("button")! as HTMLButtonElement

If you are NOT certain that the button exists in the HTML document, you can use the suggestion given by VS Code:

If you hover the mouse over document.getElementById you can see that the returned type is null | HTMLElement

I've rewritten your object as a class, because in object notation the colon is used to pass the value, not the type: const obj = {a:3}

class FormValidation {
    submitBtn: null | HTMLButtonElement

    mapDOM() {
        this.submitBtn = document.getElementById('js-btn-submit') as HTMLButtonElement;
    }
}

const f = new FormValidation()
f.mapDOM()
\$\endgroup\$
  • \$\begingroup\$ Thanks Kokodoko, the only thing is that the compiler is now complaining with "null" by saying that "Object is possibly null", and that "The right-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type". I've also checked that null as unkown | HTMLFormElement works as well! \$\endgroup\$ – Jose A Nov 1 '18 at 23:47
  • \$\begingroup\$ I see, this is because the colon : in object notation is used to pass the value, instead of the type! obj = {a:3}. So you can't define the type in this way. In my update I have rewritten the object as a class so you can define the type. \$\endgroup\$ – Kokodoko Nov 2 '18 at 8:35
  • \$\begingroup\$ If it's a class it then does make sense! You even gave me an idea! To define the object's property at the object level and not at the property level!! Thanks :) \$\endgroup\$ – Jose A Nov 2 '18 at 11:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.