Skip to content

feat: V1#7

Open
ramyareye wants to merge 8 commits into
WeAreMahsaAmini:mainfrom
ramyareye:fetcher
Open

feat: V1#7
ramyareye wants to merge 8 commits into
WeAreMahsaAmini:mainfrom
ramyareye:fetcher

Conversation

@ramyareye
Copy link
Copy Markdown
Collaborator

@ramyareye ramyareye commented Nov 27, 2022

Implementations:

  • Read and Sync with Google Sheet including provinces, cities and people entities
  • Generate local JSONs

closes #2

Copy link
Copy Markdown

@y-nk y-nk left a comment

Choose a reason for hiding this comment

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

left a bunch of comments as requested, but approving since i've no authority on this project.

congratulations for your involvement in this project.

Comment thread src/data/index.ts Outdated
import type { Person, City, Province } from 'types';

const getPeople = () => {
return people;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return people;
return people as Person[];

cast here instead of getData so that the types are also available granularly

Comment thread src/data/index.ts Outdated
};

const getProvinces = () => {
return provinces;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
return provinces;
return provinces as Province[];

Comment thread src/utils/file/file.ts
const fileUrl = (type: DataTypes) =>
process.cwd() + `/public/data/${type}.json`;

export const readFile = async (type: DataTypes) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you can use fs/promises i guess

Comment thread src/utils/file/file.ts
};

export const writeFile = (type: DataTypes, file: string) => {
fs.writeFileSync(fileUrl(type), file);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

doesn't hurt to promisify this too imho

Comment thread src/utils/file/cities.ts
cities = cities.map((ct) => (ct.id === city.id ? city : ct));
};

export const writeCities = () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if writeFile is promisified u can save a little time with a Promise.all

['F', 33],
];

const sheetRange = `Provinces!${
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this sounds pretty magical lol. i would rather go for notion api, looks less fragile.

or you can leverage this instead https://www.npmjs.com/package/google-spreadsheet

as an optimization i suggest you to used the row metadata property to store the uuid of your record. it is invisible to google spreadsheet so people cannot tamper the data, and it's more optimized when you want to fetch one row by id.

Comment thread src/utils/api.ts
{ responseType: 'stream' }
);

const fileName =
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this i really dont get it 😂

import { get as getSheetData, update as updateSheetData } from 'utils/api';

const fetch = async (req: NextApiRequest, res: NextApiResponse) => {
const data = await getSheetData('2022!D2:E300');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if (req.method !== "get") res.status(404).end() ?

@@ -0,0 +1,130 @@
import type { NextApiRequest, NextApiResponse } from 'next';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

best pattern in next.js is to avoid coding business logic inside the api route directly, otherwise you will block yourself from doing SSR.

better to do business logic in a separate function and import it, such as (simplified):

export default async function (req, res) {
  const data = zod.validate(req.query)

  try {
    const result = await myLibFn(data)
    res.status(200).json(result)
  } catch (err) {
    res.status(500).json({ err })
  }
}

}) {
const clickRef = useRef(null);
const { city, province } = getCityProvince(person.city);
const { getCity, getProvince } = useData();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

imho best would be to leverage getServerSideProps instead of using a data provider so that you can enable SSR.

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.

Why don't we put effort on this?

2 participants