Skip to content

WIP update vulnerable dependencies; update to vite #8

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rlassi
Copy link
Contributor

@rlassi rlassi commented Dec 17, 2022

0 vulnerabilities found now with npm audit. I think they were all resolved in the migration to vite. I don't have vue updated to 2.7 yet or vuetify updated. In your slack message you mentioned bumping vuetify to 2.6 but it's already there. Did you want to bump it up further?

Moving to vite from vue-cli is close. I can get the project up, but I am getting 503 errors from server and I think it has something to do with my proxy... or something else in the vite.config.js setup. Can you take a look and see if you can resolve the issue? npm run dev will start the dev server. LMK if you have any questions or feedback.

I referenced this for vue-cli to vite migration - https://vueschool.io/articles/vuejs-tutorials/how-to-migrate-from-vue-cli-to-vite/

@rlassi rlassi requested a review from kchapple December 17, 2022 05:47
@rlassi rlassi force-pushed the update-vulnerable-packages-plus-vue-vuetify-vite branch from 84f2f70 to aca6a40 Compare December 17, 2022 05:51
export default defineConfig({
plugins: [vue()],
resolve: {
alias: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if you are using an @ alias but it was part of the vite config setup steps so I figured I'd throw it in.

alias: {
"@": path.resolve(__dirname, "./src"),
},
extensions: [".mjs", ".js", ".ts", ".jsx", ".tsx", ".json", ".vue"], // allows importing files without adding file extension
Copy link
Contributor Author

@rlassi rlassi Dec 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to vite this is actually not recommended. Vite recommends changing all vue sfc's (single file components) to have .vue extension on imports. ex - instead of import MyComponent from './MyComponent' you would need it to be import MyComponent from './MyComponent.vue'. However if the app is large and it would be a big ask to add the extension this is a work around. I didn't look into it, but there may be a migration tool that will rename imports accordingly...

vite.config.js Outdated
},
},
host: "127.0.0.1",
proxy: { "/": "http://localhost:8300" },
Copy link
Contributor Author

@rlassi rlassi Dec 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is where the openemr app is not connecting properly to the vue app and is causing the 503 codes from server. I can do some more debugging of this, but I figured I'd get your thoughts on it first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the dev server is starting on a different port now (not 8080) and the docker apache configuration has a reverse proxy to 8080. Is there a way to force vite to start devserver on 8080?

The apache conf is here (on the openemr Docker):
/etc/apache2/conf.d/openemr.conf

It might be helpful debugging the proxy to add these configs

proxy: {
      '/' : {
        target: "127.0.0.1:8300",
        changeOrigin: true,
        secure: false,
        configure: (proxy, _options) => {
          proxy.on('error', (err, _req, _res) => {
            console.log('proxy error', err);
          });
          proxy.on('proxyReq', (proxyReq, req, _res) => {
            console.log('Sending Request to the Target:', req.method, req.url);
          });
          proxy.on('proxyRes', (proxyRes, req, _res) => {
            console.log('Received Response from the Target:', proxyRes.statusCode, req.url);
          });
        },
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a great idea. I'll add that in. I think I remember reading that there is a port key you can specify a custom port for the dev server. I'll check into it.

@@ -1,3 +1,3 @@
VUE_APP_BASE_URL=/openemr
VUE_APP_API_BASE_URL=/openemr
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove these env changes from final commit before merging...

@@ -1,9 +1,9 @@
import axios from 'axios'
import { newDashboard, newDataSourceColumn, newForm, newWorkspace } from './model-builder'

const baseUrl = process.env.VUE_APP_API_BASE_URL
const baseUrl = import.meta.VITE_API_BASE_URL
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vite uses import.meta for env vars, and VITE_ instead of VUE_APP_ to signify client side env vars

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants