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

Update node version and NPM packages #78

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

Souptik2001
Copy link
Member

Issue - #71

This PR -

  • Updates the node version.
  • Updates all the NPM packages.
  • Updates the codebase to fix errors generated by the newer versions of the packages.
  • Removes the PWA integration as the package used currently (next-offline) has not been updated since last 2 years. We will be integrating it later with maybe next-pwa package. Reference issue.

Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
Because the current next-offline package has not been updated since 2
years. We will be implementing it later using probably next-pwa package.

Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
@Souptik2001 Souptik2001 self-assigned this Feb 3, 2023
@vercel
Copy link

vercel bot commented Feb 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
wp-decoupled ❌ Failed (Inspect) Feb 18, 2023 at 11:40AM (UTC)

Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
.eslintrc.js Outdated Show resolved Hide resolved
import Link from 'next/link';
import client from '../src/apollo/ApolloClient';
import AddToCartButton from '../src/components/cart/AddToCartButton';
import Hero from '../src/components/home/Hero';
import Image from '../src/components/Image';
import Layout from '../src/components/layouts/Layout';
Copy link
Contributor

Choose a reason for hiding this comment

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

@Souptik2001 Let's segregate these with comments, Internal dependencies and external dependencies resp. Lets do that where applicable.

Copy link
Member Author

Choose a reason for hiding this comment

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

@imranhsayed Just to be once confirmed you meant like this right? -

// External.
import Link from 'next/link';

// Internal.
import client from '../src/apollo/ApolloClient';
import AddToCartButton from '../src/components/cart/AddToCartButton';
import Hero from '../src/components/home/Hero';
import Image from '../src/components/Image';
import Layout from '../src/components/layouts/Layout';

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly @Souptik2001 Looks more organised.

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Souptik2001 When we assign the issue back for review, it's the reviewer who will mark the comments resolved after checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Souptik2001 Let's rename the comments to Internal Dependencies and External Dependencies respectively.

] = useMutation(LOGIN_USER, { client })
const [login, { data: data, loading: loading, error: error }] = useMutation(LOGIN_USER, {
client
});
Copy link
Contributor

Choose a reason for hiding this comment

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

@Souptik2001 Let's break it into multiple lines to make it more readable

Copy link
Member Author

Choose a reason for hiding this comment

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

@imranhsayed As per the current prettier configuration, as this line doesn't exceed a threshold line length limit, that's why it is showing error if we are breaking this line.

So, should we change that configuration, because otherwise there will be linting error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please. let's update the configuration to make it look more readable @Souptik2001

Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
to distinguish between external and internal imports.

Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
{totalPrice ? (
<span className="wp-cart-price mr-2">${totalPrice.toFixed(2)}</span>
) : (
''
Copy link
Contributor

Choose a reason for hiding this comment

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

@Souptik2001 Let's update all the '' empty quotes to null

@thelovekesh
Copy link
Member

@Souptik2001 @imranhsayed Just FYI, I am putting this through in #80.

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.

3 participants