13577 Cleanup before upgrade #443

severinbeauvais · 2022-09-13T20:10:33Z

Description of changes:

pre-upgrade (lint and other misc cleanup)
app version = 4.2.1

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).

severinbeauvais · 2022-09-13T20:14:56Z

package-lock.json

-      "version": "7.2.5",
-      "resolved": "https://registry.npmjs.org/vue-class-component/-/vue-class-component-7.2.5.tgz",
-      "integrity": "sha512-0CSftHY0bDTD+4FbYkuFf6+iKDjZ4h2in2YYJDRMk5daZIjrgT9LjFHvP7Rzqy9/s1pij3zDtTSLRUjsPWMwqg==",
+      "version": "7.2.6",


This is a normal patch upgrade, seen here because I totally rebuild this package lock file. It should have no effect.

severinbeauvais · 2022-09-13T20:18:30Z

src/App.vue

@@ -444,7 +446,7 @@ export default class App extends Mixins(
  }

  /** Called when component is created. */
-  async created (): Promise<void> {
+  protected async created (): Promise<void> {


created() should not be public.

severinbeauvais · 2022-09-13T20:18:57Z

src/App.vue

@@ -498,7 +500,7 @@ export default class App extends Mixins(
  }

  /** Called when component is destroyed. */
-  private destroyed (): void {
+  protected beforeDestroy (): void {


This is the new lifecycle event name in Vue3 but it's backward-compatible with Vue2.

severinbeauvais · 2022-09-13T20:20:14Z

src/components/Dissolution/AssociationDetails.vue

@@ -161,12 +161,12 @@ export default class AssociationDetails extends Mixins(CommonMixin, EnumMixin, D
  }

  /** Whether the address object is empty. */
-  private isEmptyAddress (address: AddressIF): boolean {
+  protected isEmptyAddress (address: AddressIF): boolean {


isEmptyAddress() needs to be protected so the template (HTML) can call it. (This is not yet enforced by the framework but this is correct and the previous code was incorrect.)

severinbeauvais · 2022-09-13T20:28:25Z

src/components/Dissolution/CooperativeType.vue

  @Prop({ default: false }) readonly showErrors!: boolean

  // Global getter
  @Getter getCooperativeType!: CoopTypes

  // Local properties
-  private readonly items: Array<any> = [
+  readonly items: Array<any> = [


This is safe without being private because external code still can't change it.

severinbeauvais · 2022-09-13T20:32:03Z

src/components/Incorporation/IncorporationDateTime.vue


  // Date Time Selectors
-  private hours: Array<string> = [...Array(12).keys()]
+  readonly hours: Array<string> = [...Array(12).keys()]
    .map(num => (++num).toLocaleString('en-US'))


This is a "constant" (but needs to be declared as readonly because it's a class member).

These variables don't change. One more way of saying it is: they are static.

severinbeauvais · 2022-09-13T20:33:10Z

src/components/Incorporation/IncorporationDateTime.vue

@@ -152,21 +154,21 @@ export default class IncorporationDateTime extends Mixins(DateMixin) {
  }

  /** The array of validations rules for effective date hours field. */
-  private get hourRules (): Array<Function> {
+  get hourRules (): Array<Function> {
    return [
      v => this.dateText !== '' && (/^([1-9]|1[012])$/.test(v) || '')


This cannot be a readonly array because it uses a variable in its code. So it's a getter instead.

severinbeauvais · 2022-09-13T20:34:09Z

src/components/Incorporation/IncorporationDateTime.vue

    const date = new Date(this.getCurrentJsDate) // make a copy
    date.setDate(date.getDate() + 10)
    return this.dateToYyyyMmDd(date)
  }

  /** The minimum time that can be entered (now + 3 minutes). */
-  private minTime (): string {
+  protected minTime (): string {
    const date = this.getCurrentJsDate
    return new Date(date.getTime() + 180000)
      .toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit', hour12: true })
  }


This could probably be a getter instead of a function but changing it is really out of scope for this cleanup.

severinbeauvais · 2022-09-13T20:37:25Z

src/components/Registration/BusinessTypeConfirm.vue

@@ -53,14 +53,14 @@ export default class BusinessTypeConfirm extends Vue {
  protected checked = false
  protected label = 'General Partnership'
  protected labelSP = 'BC Sole Proprietorship / Doing Business As (DBA) Registration'
-  protected text = `I acknowledge that a General Partnership cannot be \
-    changed into a Sole Proprietorship (including DBA). If this is \


The latest ES Lint tried to reformat this in a way that's incompatible with our current framework so I manually reformatted this into something ES Lint likes.

severinbeauvais · 2022-09-13T20:38:22Z

src/components/common/BusinessContactInfo.vue

@@ -113,8 +113,8 @@ export default class BusinessContactInfo extends Mixins(CommonMixin) {
  // Rules for template
  readonly Rules = Rules

-  private contact: ContactPointIF = this.initialValue
-  private formValid: boolean = false
+  protected contact = this.initialValue


contact gets its type from initialValue.

severinbeauvais · 2022-09-13T20:39:24Z

src/components/common/NameRequestInfo.vue

@@ -104,7 +104,7 @@
              class="pt-0 mt-0"
              v-model="hasNameTranslation"
              id="name-translation-checkbox"
-              @click.native="confirmNameTranslation()"


This is not supported in Vue3 but I tested this and this code works correctly without it.

severinbeauvais · 2022-09-13T20:41:42Z

src/components/common/OfficeAddresses.vue

-  // State of the checkbox for determining whether the Record address is the same as the Registered address
-  private inheritRegisteredAddress: boolean = true
+  /** State of the checkbox for determining whether the Record address is the same as the Registered address. */
+  private inheritRegisteredAddress = true


The comment in JS Doc format allows VS Code to display the comment when hovering over the variable.

severinbeauvais · 2022-09-13T20:45:28Z

src/main.ts

@@ -43,17 +44,17 @@ async function start () {
  // must come first as inits below depend on config
  await fetchConfig()

-  if (window['sentryEnable'] === 'true') {
+  if ((window as any).sentryEnable === 'true') {


ES Lint wanted to change this to window.sentryEnable but the window object doesn't have that property and this caused an error.

The correct to fix this is to extend the window object to declare any additional properties ... I'm calling that out of scope for this cleanup.

severinbeauvais · 2022-09-13T21:04:04Z

tests/unit/DissolutionStatement.spec.ts

@@ -5,10 +5,9 @@ import { CorpTypeCd, DissolutionStatementTypes } from '@/enums'
 import { DissolutionResources } from '@/resources'

 // Input field selectors to test changes to the DOM elements.
-const typeSelector: string = '#dissolution-statement-'
-const summaryErrorSelector: string = '.invalid-section'


Wasn't used.

severinbeauvais · 2022-09-13T21:32:13Z

src/interfaces/store-interfaces/state-interfaces/name-request-interface.ts

-  details: {},
-  applicant: {},
+  details: {} as NameRequestDetailsIF,
+  applicant: {} as NameRequestApplicantIF,


These should probably be null and handled accordingly in code but that's out of scope for this cleanup.

What I did here is remove the {} union type from details to pre-empt an error where details.approvedName was not recognized as valid.

As far as I can tell, the code runs normally atm.

severinbeauvais · 2022-09-13T21:32:48Z

src/mixins/filing-template-mixin.ts

@@ -368,7 +369,7 @@ export default class FilingTemplateMixin extends Mixins(DateMixin) {
            : {}
        },
        nameRequest: {
-          legalName: this.getNameRequest.details['approvedName'],


This was a sloppy way to work around a type issue.

Using (myObject as any).myProperty is also not ideal, but lint can flag variables used as any so we could find and fix these in the future. Plus there aren't a lot of places where "as any" is used 😅

severinbeauvais · 2022-09-13T21:34:50Z

tests/unit/FileUploadPreview.spec.ts

-  }
-  await Vue.nextTick()
-}
-


I moved this to its own TS file so it can be imported in a couple of test suites (instead of duplicated).

severinbeauvais · 2022-09-13T21:48:43Z

@seeker25 @vysakh-menon-aot @cameron-freshworks @lekshmimallika-aot

Please note the updates made by the next version of ES Lint, which I fixed here pre-emptively. Please follow this code style from now on (to prevent future cleanup).

lewischenstudio

This is a lot of cleanups. Thanks.

- app version = 4.2.1

severinbeauvais self-assigned this Sep 13, 2022

severinbeauvais force-pushed the 13577 branch from dec7557 to b907ce8 Compare September 13, 2022 20:13

severinbeauvais commented Sep 13, 2022

View reviewed changes

severinbeauvais force-pushed the 13577 branch from 09a7071 to 70015da Compare September 13, 2022 20:17

severinbeauvais commented Sep 13, 2022

View reviewed changes

severinbeauvais requested review from lewischenstudio and lambert-alex September 13, 2022 20:21

severinbeauvais force-pushed the 13577 branch from 43efdf0 to 40951d0 Compare September 13, 2022 20:27

severinbeauvais commented Sep 13, 2022

View reviewed changes

severinbeauvais force-pushed the 13577 branch from 40951d0 to e18b252 Compare September 13, 2022 21:27

severinbeauvais commented Sep 13, 2022

View reviewed changes

severinbeauvais force-pushed the 13577 branch from e18b252 to 3e05593 Compare September 13, 2022 21:46

lewischenstudio approved these changes Sep 13, 2022

View reviewed changes

- pre-upgrade (lint and other misc cleanup)

3355305

- app version = 4.2.1

severinbeauvais force-pushed the 13577 branch from 3e05593 to 3355305 Compare September 14, 2022 00:01

severinbeauvais merged commit c1afbed into bcgov:main Sep 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

13577 Cleanup before upgrade #443

13577 Cleanup before upgrade #443

severinbeauvais commented Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022 •

edited

Loading

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022 •

edited

Loading

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022 •

edited

Loading

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais Sep 13, 2022

severinbeauvais commented Sep 13, 2022

lewischenstudio left a comment

13577 Cleanup before upgrade #443

13577 Cleanup before upgrade #443

Conversation

severinbeauvais commented Sep 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

severinbeauvais Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

severinbeauvais Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

severinbeauvais Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

severinbeauvais commented Sep 13, 2022

lewischenstudio left a comment

Choose a reason for hiding this comment

severinbeauvais Sep 13, 2022 •

edited

Loading

severinbeauvais Sep 13, 2022 •

edited

Loading

severinbeauvais Sep 13, 2022 •

edited

Loading