Skip to content

Dev#1

Open
teenurani wants to merge 88 commits intomasterfrom
dev
Open

Dev#1
teenurani wants to merge 88 commits intomasterfrom
dev

Conversation

@teenurani
Copy link
Collaborator

@teenurani teenurani commented Oct 10, 2019

const config = require("./webpack.config");
const compiler = webpack(config);

const webpackDevMiddleware = require("webpack-dev-middleware")(

Choose a reason for hiding this comment

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

But why still you need it? Isn't it picking up automatically?

.env Outdated
@@ -0,0 +1,2 @@
URL=http://localhost:3000

Choose a reason for hiding this comment

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

Please replace it with BASE_DOMAIN. Also, there should be different env files for each env.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done, targeting only dev right now.

.babelrc Outdated
"env",
{
"targets": {
"browsers": ["last 2 Chrome versions"]

Choose a reason for hiding this comment

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

Why only chrome versions? It should support other browser as well.

.babelrc Outdated
{
"presets": [
[
"env",

Choose a reason for hiding this comment

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

@babel/preset-env should be used

src/main.js Outdated
@@ -0,0 +1,3 @@
require("webpack-hot-middleware/client");

Choose a reason for hiding this comment

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

why have you used this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using webpack dev server. Both running on different port. Through this everything running on single port.

src/main.js Outdated
@@ -0,0 +1,3 @@
require("webpack-hot-middleware/client");
require("./main.scss");

Choose a reason for hiding this comment

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

why do we need require? It should be import.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done


const cartListData = { data: [] };

products.forEach(function(product) {

Choose a reason for hiding this comment

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

use map instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

router.post("/removeFromCart", function(req, res) {
const index = cart.indexOf(req.body.productId);
if (index !== -1) cart.splice(index, 1);
addToCart.cart = cart.length;

Choose a reason for hiding this comment

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

is there a reason to use cart.length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, checking no of items.

try {
const cart = await addToCarts(productId);
// render result on UI
CART_SELECTOR.cartItem.textContent = cart.cart + " items";

Choose a reason for hiding this comment

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

can we use more descriptive name then cart.cart?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

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.

4 participants