-
Notifications
You must be signed in to change notification settings - Fork 2
[fix] 'route/:id' refresh issue by removing explicit 'base' from vite… #140
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -3,54 +3,67 @@ import { defineConfig } from "vite"; | |||
import react from "@vitejs/plugin-react"; | ||||
import dotenv from "dotenv"; | ||||
import eslintPlugin from "vite-plugin-eslint"; | ||||
import { resolve } from "path"; | ||||
import { readFileSync } from "fs"; | ||||
|
||||
dotenv.config(); | ||||
|
||||
export default defineConfig({ | ||||
base: "", | ||||
root: "", | ||||
envPrefix: "RECORD_MANAGER_", | ||||
plugins: [ | ||||
react(), | ||||
eslintPlugin({ | ||||
cache: false, | ||||
}), | ||||
], | ||||
build: { | ||||
sourcemap: true, | ||||
emptyOutDir: true, | ||||
rollupOptions: { | ||||
onwarn: (warning, defaultHandler) => { | ||||
// TODO: workaround for dangerous use of eval method that should be solved in SForms library issue https://github.com/kbss-cvut/s-forms/issues/283 | ||||
const isFromKbssCvutPackageWarning = | ||||
warning.code === "EVAL" && warning.message.includes("node_modules/store/plugins/lib/json2.js"); | ||||
// TODO: Rollup Pure Annotation warning should be resolved by https://github.com/kbss-cvut/s-forms/issues/282 | ||||
const isRollupPureAnnotationWarning = | ||||
warning.code === "INVALID_ANNOTATION" && warning.message.includes("*#__PURE__*"); | ||||
if (isFromKbssCvutPackageWarning || isRollupPureAnnotationWarning) { | ||||
return; | ||||
} | ||||
defaultHandler(warning); | ||||
function loadEnv(mode) { | ||||
const envPath = resolve(process.cwd(), `.env.${mode}`); | ||||
const envContent = readFileSync(envPath, "utf-8"); | ||||
const env = dotenv.parse(envContent); | ||||
return env; | ||||
} | ||||
|
||||
export default ({ mode }) => { | ||||
const env = loadEnv(mode); | ||||
|
||||
return defineConfig({ | ||||
base: env.RECORD_MANAGER_BASENAME, | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might not understand how it works, but this seems to me strange to use. I thought we had environments set up in record-manager-ui/config/index.js Line 33 in 9f44702
So I am not sure how this setting is useful for us. I thought we would set Router basename instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume (but did not check) |
||||
root: "", | ||||
envPrefix: "RECORD_MANAGER_", | ||||
plugins: [ | ||||
react(), | ||||
eslintPlugin({ | ||||
cache: false, | ||||
}), | ||||
], | ||||
build: { | ||||
sourcemap: true, | ||||
emptyOutDir: true, | ||||
rollupOptions: { | ||||
onwarn: (warning, defaultHandler) => { | ||||
// TODO: workaround for dangerous use of eval method that should be solved in SForms library issue https://github.com/kbss-cvut/s-forms/issues/283 | ||||
const isFromKbssCvutPackageWarning = | ||||
warning.code === "EVAL" && warning.message.includes("node_modules/store/plugins/lib/json2.js"); | ||||
// TODO: Rollup Pure Annotation warning should be resolved by https://github.com/kbss-cvut/s-forms/issues/282 | ||||
const isRollupPureAnnotationWarning = | ||||
warning.code === "INVALID_ANNOTATION" && warning.message.includes("*#__PURE__*"); | ||||
if (isFromKbssCvutPackageWarning || isRollupPureAnnotationWarning) { | ||||
return; | ||||
} | ||||
defaultHandler(warning); | ||||
}, | ||||
}, | ||||
cssMinify: false, // TODO: workaround for CSS syntax error from SForms library that should be resolved by https://github.com/kbss-cvut/s-forms/issues/283 | ||||
}, | ||||
cssMinify: false, // TODO: workaround for CSS syntax error from SForms library that should be resolved by https://github.com/kbss-cvut/s-forms/issues/283 | ||||
}, | ||||
define: { | ||||
"process.env": process.env, // workaround for parse-link-header library that depends on 2 vars defined in `process.env`, see https://github.com/thlorenz/parse-link-header/issues/31 | ||||
}, | ||||
resolve: { | ||||
alias: { | ||||
querystring: "querystring-es3", // workaround for parse-link-header library that replaces nodejs builtin module with the module adapted for browser | ||||
url: "url-parse", // workaround for parse-link-header library that replaces nodejs builtin module with the module adapted for browser | ||||
define: { | ||||
"process.env": process.env, // workaround for parse-link-header library that depends on 2 vars defined in `process.env`, see https://github.com/thlorenz/parse-link-header/issues/31 | ||||
}, | ||||
resolve: { | ||||
alias: { | ||||
querystring: "querystring-es3", // workaround for parse-link-header library that replaces nodejs builtin module with the module adapted for browser | ||||
url: "url-parse", // workaround for parse-link-header library that replaces nodejs builtin module with the module adapted for browser | ||||
}, | ||||
}, | ||||
}, | ||||
test: { | ||||
environment: "jsdom", | ||||
setupFiles: ["./tests/setup.js"], | ||||
server: { | ||||
deps: { | ||||
inline: ["@kbss-cvut/s-forms"], | ||||
test: { | ||||
environment: "jsdom", | ||||
setupFiles: ["./tests/setup.js"], | ||||
server: { | ||||
deps: { | ||||
inline: ["@kbss-cvut/s-forms"], | ||||
}, | ||||
}, | ||||
}, | ||||
}, | ||||
}); | ||||
}); | ||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is motivation to change this?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blcham Because I thought of every environment (dev, production) having it's own base in vite.config.js which will use the value of the RECORD_MANAGER_BASENAME based on mode. So the dev environment will have a "/" base, and the production will have a "/record-manager" base . Since the build for production image is then used in dockerization, it means that the base of the build (in vite.config.js) should match with the BASENAME passed via docker (which will be /record-manager, since RECORD_MANAGER_ROOT_PATH in deploy > internal-auth > .env is commented out). https://reactrouter.com/en/main/router-components/router
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what if i decide to have
RECORD_MANAGER_ROOT_PATH=/my-record-manager
?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blcham It doesn't work after I changed it, I feel stuck now