Skip to content
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

Feature/vite migration #94

Closed
wants to merge 11 commits into from

Conversation

shellyear
Copy link
Collaborator

@shellyear shellyear commented Feb 21, 2024

Resolves: #88

Zistili jsme:

  • kdyz v dist/config.js nastavis promenne tak jako by byli substituovane pomoci docker-compose.yml tak pak odtestovat docker znamena odtestovat "npm run preview"

@blcham
Copy link

blcham commented Feb 21, 2024

@blcham @LaChope

@blcham blcham self-requested a review February 21, 2024 13:14
import PropTypes from "prop-types";

let HistoryPagination = (props) => (
<nav className="d-flex justify-content-center">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think flexbox is much better than bootstrap but for consistency with the rest of the app I assume bootstrap should be used

Copy link

@blcham blcham left a comment

Choose a reason for hiding this comment

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

See the comments

config/index.js Outdated
@@ -1,31 +1,36 @@
/**
* Aggregated object of process.env and window.__config__ to allow dynamic configuration
*/

const ENV_KEY_BASE = "RECORD_MANAGER_";
Copy link

@blcham blcham Feb 21, 2024

Choose a reason for hiding this comment

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

"build": "webpack --config ./webpack.config.js",
"preview": "vite preview",
"start": "vite",
"build": "vite build",
"build:static": "cross-env NODE_ENV=production STATIC=true webpack --env.prod -p --config ./webpack.config.js",
Copy link

Choose a reason for hiding this comment

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

build:static is not relevant anymore

@@ -70,14 +72,17 @@
"querystring-es3": "^0.2.1",
"redux-mock-store": "^1.5.4",
"style-loader": "^1.1.3",
"vite": "^5.1.2",
"vite-plugin-simple-html": "^0.1.2",
Copy link

Choose a reason for hiding this comment

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

Why this ?

"start": "webpack serve --config ./webpack.config.js",
"build": "webpack --config ./webpack.config.js",
"preview": "vite preview",
"start": "vite",
Copy link

Choose a reason for hiding this comment

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

Suggested change
"start": "vite",
"dev": "vite",

},
plugins: [
pluginReact(),
simpleHtmlPlugin({
Copy link

Choose a reason for hiding this comment

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

We do not need this ... it is configured by config.js

},
plugins: [
pluginReact(),
simpleHtmlPlugin({
Copy link

Choose a reason for hiding this comment

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

Can we use config.js/.ENV for configuring it?

),
"import.meta.env.RECORD_MANAGER_API_URL": JSON.stringify(env.VITE_RECORD_MANAGER_API_URL),
"import.meta.env.RECORD_MANAGER_APP_TITLE": JSON.stringify(env.VITE_RECORD_MANAGER_APP_TITLE),
"import.meta.env.RECORD_MANAGER_DEV_SERVER_PORT": JSON.stringify(env.VITE_RECORD_MANAGER_DEV_SERVER_PORT),
Copy link

Choose a reason for hiding this comment

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

we want to remove that and use https://vitejs.dev/config/shared-options.html#envprefix RECORD_MANAGER_

@@ -0,0 +1,32 @@
import React from "react";
Copy link

@blcham blcham Feb 21, 2024

Choose a reason for hiding this comment

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

We need history of this file and other files that were just moved. Move in git instead of remove + create.

@blcham
Copy link

blcham commented Feb 29, 2024

Closing in favor of akaene#4

@blcham blcham closed this Feb 29, 2024
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.

Migrate from Webpack to Vite
3 participants