17165 Migrated from Vue CLI to Vite #524
Conversation
|
/gcbrun |
|
Temporary Url for review: https://business-edit-dev--pr-524-sru298p2.web.app |
| @@ -20,7 +20,7 @@ | |||
| { "key" : "X-XSS-Protection", "value" : "1; mode=block" }, | |||
| { | |||
| "key": "Content-Security-Policy", | |||
| "value": "default-src 'self'; frame-src 'self' *.gov.bc.ca *.hotjar.com *.googleapis.com https://*.nr-data.net https://*.newrelic.com https://*.cac1.pure.cloud; script-src 'self' 'unsafe-eval' 'unsafe-inline' *.gov.bc.ca *.hotjar.com *.googleapis.com https://*.nr-data.net https://*.newrelic.com https://*.cac1.pure.cloud; style-src 'self' 'unsafe-inline' *.cloudflare.com *.googleapis.com; font-src 'self' *.gov.bc.ca *.hotjar.com *.cloudflare.com *.googleapis.com *.gstatic.com *.jsdelivr.net; img-src 'self' data: *.hotjar.com https://*.cac1.pure.cloud; connect-src 'self' *.gov.bc.ca *.launchdarkly.com *.hotjar.com *.postescanada-canadapost.ca *.sentry.io *.apigee.net wss://*.hotjar.com *.hotjar.io https://*.nr-data.net https://shyrka-prod-cac1.s3.ca-central-1.amazonaws.com https://*.newrelic.com https://*.cac1.pure.cloud wss://*.cac1.pure.cloud; manifest-src 'self'; media-src 'self' https://*.cac1.pure.cloud; object-src 'self' https://*.cac1.pure.cloud; child-src 'self' https://*.cac1.pure.cloud;" | |||
| "value": "default-src 'self'; frame-src 'self' *.gov.bc.ca *.hotjar.com *.googleapis.com https://*.nr-data.net https://*.newrelic.com https://*.cac1.pure.cloud; script-src 'self' 'unsafe-eval' 'unsafe-inline' *.gov.bc.ca *.hotjar.com *.googleapis.com https://*.nr-data.net https://*.newrelic.com https://*.cac1.pure.cloud; style-src 'self' 'unsafe-inline' *.cloudflare.com *.googleapis.com; font-src 'self' *.gov.bc.ca *.hotjar.com *.cloudflare.com *.googleapis.com *.gstatic.com *.jsdelivr.net; img-src 'self' *.postescanada-canadapost.ca https://*.cac1.pure.cloud data: *.hotjar.com https://*.cac1.pure.cloud; connect-src 'self' *.gov.bc.ca *.launchdarkly.com *.hotjar.com *.postescanada-canadapost.ca *.sentry.io *.apigee.net wss://*.hotjar.com *.hotjar.io https://*.nr-data.net https://shyrka-prod-cac1.s3.ca-central-1.amazonaws.com https://*.newrelic.com https://*.cac1.pure.cloud wss://*.cac1.pure.cloud; manifest-src 'self'; media-src 'self' https://*.cac1.pure.cloud; object-src 'self' https://*.cac1.pure.cloud; child-src 'self' https://*.cac1.pure.cloud;" | |||
There was a problem hiding this comment.
Fixing the Canada post icon:
The same change has been done in the nginx.conf file.
It works now:
There was a problem hiding this comment.
Does this have to be fixed in Create UI also? I only see 1 ref to CanadaPost in Create UI but 2 here.
There was a problem hiding this comment.
No Sev. It already works fine in Create UI. In nginx.conf, there should be 2 references for it. I went ahead and made the line similar in firebase.json because they should be similar.
In Create UI, it's the same as here in nginx.conf now but it's little bit different in firebase.json like you mentioned as there's only one ref there. In my opinion, we should change in Create UI so that the CSP is similar in both files. However, this change won't affect any functionalities. It's just to stay consistent.
There was a problem hiding this comment.
OK, thanks. I have no further comment on this.
| }, | ||
| extensions: ['.mjs', '.js', '.ts', '.jsx', '.tsx', '.json', '.vue'], | ||
| // Fix inline dependency of a dependency, which is the case in Tiptap-Vuetify | ||
| mainFields: ['module'] |
There was a problem hiding this comment.
Tiptap Vuetify wasn't letting my tests to run:
This is to fix that.
| "@bcrs-shared-components/court-order-poa": "2.1.3", | ||
| "@bcrs-shared-components/date-picker": "1.2.5", | ||
| "@bcrs-shared-components/detail-comment": "1.1.1", | ||
| "@bcrs-shared-components/enums": "1.0.45", | ||
| "@bcrs-shared-components/detail-comment": "1.1.3", |
There was a problem hiding this comment.
Updating detail comment component to fix lodash issue.
| esbuildOptions: { | ||
| // Fix Module has been externalized for browser compatibility warning. | ||
| plugins: [ | ||
| NodeModulesPolyfillPlugin() |
There was a problem hiding this comment.
There was a warning in the console ONLY when running locally:
This fixes all of them except for source-map-js warning. When running locally now:
|
/gcbrun |
|
Temporary Url for review: https://business-edit-dev--pr-524-sru298p2.web.app |
JazzarKarim
requested review from
severinbeauvais,
seeker25,
ketaki-deodhar and
leodube-aot
| "test:debug": "node --inspect-brk node_modules/.bin/vue-cli-service test:unit --testPathPattern --no-cache --watch --runInBand" | ||
| "dev": "vite", | ||
| "build": "vite build", | ||
| "serve": "vite preview", |
There was a problem hiding this comment.
port 8080? maybe not call it serve (otherwise someone on autopilot will be like why isn't my code changing like before? )
I called it preview
There was a problem hiding this comment.
Makes a lot sense. I was just following the official guide in this and the other PR: https://vueschool.io/articles/vuejs-tutorials/how-to-migrate-from-vue-cli-to-vite/
In the guide, they mention that npm run serve is vite preview.
I do agree with you though Travis.
There was a problem hiding this comment.
Yeah, I was bitten by this last week. I have to condition myself to type "npm run dev" now, that's all.
I like that Karim is following the official guide.
package.json
Outdated
| "vue-hotjar": "^1.4.0", | ||
| "vue-router": "^3.6.5", | ||
| "vue-the-mask": "^0.11.1", | ||
| "vue2-filters": "^0.14.0", | ||
| "vuelidate": "^0.7.7", | ||
| "vuelidate": "0.6.2", | ||
| "vuetify": "^2.6.15", | ||
| "vuex": "^3.6.2" | ||
| }, | ||
| "devDependencies": { | ||
| "@babel/plugin-proposal-private-methods": "^7.18.6", |
There was a problem hiding this comment.
Probably don't need this babel line
| @@ -24,6 +24,7 @@ import mockRouter from './MockRouter' | |||
| import { createPinia, setActivePinia } from 'pinia' | |||
| import { useStore } from '@/store/store' | |||
| import { CorpTypeCd, FilingTypes } from '@/enums' | |||
| import { vi } from 'vitest' | |||
There was a problem hiding this comment.
I think there's a way to include vi without needing to explicitly include it in the test? don't know how though
There was a problem hiding this comment.
I think we can achieve that by using vitest.xxx and in this case, no need to import vi. Shall I make this change Travis?
There was a problem hiding this comment.
I let this one past when you did Create UI -- I don't care, either way.
| expect(labels.at(1).text()).toBe('Mailing Address Corrected') | ||
| expect(labels.at(2).text()).toBe('Delivery Address Corrected') | ||
| expect(labels.at(1).text()).toContain('Mailing Address') | ||
| expect(labels.at(1).text()).toContain('Corrected') |
There was a problem hiding this comment.
I've seen some funky stuff happen calling this twice
There was a problem hiding this comment.
Really? Should work.
| const sbcVersion = JSON.parse(packageJson).dependencies['sbc-common-components'] | ||
| const aboutText1 = (appName && appVersion) ? `${appName} v${appVersion}` : '' | ||
| const aboutText2 = (sbcName && sbcVersion) ? `${sbcName} v${sbcVersion}` : '' | ||
|
|
||
| module.exports = { |
There was a problem hiding this comment.
Project runs but getting a fatal error when I run unit tests:
There was a problem hiding this comment.
This file looks the same as in Create UI so I'm OK with this.
JazzarKarim
force-pushed
the
17165-upgrade-to-vite
branch
from
bdb7876 to
e411b68
Compare
|
/gcbrun |
|
Temporary Url for review: https://business-edit-dev--pr-524-sru298p2.web.app |
|
/gcbrun |
|
Temporary Url for review: https://business-edit-dev--pr-524-sru298p2.web.app |
severinbeauvais
changed the title
Issue #: /bcgov/entity#17165
Description of changes:
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).