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

feat(cli): get latest dependencies versions from npm #77

Merged
merged 1 commit into from Sep 26, 2019

Conversation

eladav
Copy link
Contributor

@eladav eladav commented Sep 25, 2019

@eladav
Copy link
Contributor Author

eladav commented Sep 25, 2019

Couldn't find any way to get versions for multiple packages with one request to npm APIs, and I couldn't find any alternative that supports it as well... I followed the instructions here for the metadata request:
https://github.com/npm/registry/blob/master/docs/responses/package-metadata.md

Copy link
Contributor

@EladBezalel EladBezalel left a comment

Choose a reason for hiding this comment

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

How fast is it?

@@ -9,7 +8,7 @@ const dirname = process
.pop() as string;

export default () =>
execa('npm', ['run', 'plop', 'component', process.cwd(), uppercamelcase(dirname)], {
execa('npm', ['run', 'plop', 'component', process.cwd(), dirname], {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do lower tho?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure... at this point it just shows the name of the folder... I'm running lower in the name property in package.json file... I'm considering whether or not we should auto kebab-case it...?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah I think it's fine to leave it be

import fetch from 'node-fetch';

const getDependencyMetadata = async (name: string) =>
fetch(`https://registry.npmjs.org/${name}`, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove async

}).then(r => r.json());

export const getVersionsForDependencies = async (deps: string[]) => {
return (await Promise.all(deps.map(d => getDependencyMetadata(d)))).reduce(
Copy link
Contributor

Choose a reason for hiding this comment

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

One line


export const getVersionsForDependencies = async (deps: string[]) => {
return (await Promise.all(deps.map(d => getDependencyMetadata(d)))).reduce(
(soFar: Record<string, string>, current: any) => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Why any?

@eladav
Copy link
Contributor Author

eladav commented Sep 25, 2019

matter of seconds on WiFi...

@eladav eladav force-pushed the feat-get-latest-deps-versions branch from f4d8e96 to f3714df Compare September 25, 2019 15:31
const { dependencies, devDependencies, peerDependencies } = JSON.parse(
plop.renderString(readFileSync(`./plop/component/${data.framework}/package.hbs`, 'utf-8'), data)
);
return await getVersionsForDependencies(
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need async await here

}).then(r => r.json());

export const getVersionsForDependencies = async (deps: string[]) =>
(await Promise.all(deps.map(d => getDependencyMetadata(d)))).reduce(
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here do we have to use async await?
Returning promise is enough imo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I can ‘reduce’ on the promise.all result to a name:version object

* Read package file and get versions for required dependencies
* Lowercase package name
@eladav eladav force-pushed the feat-get-latest-deps-versions branch from f3714df to c1c4729 Compare September 26, 2019 07:55
@eladav eladav merged commit ef78a1b into master Sep 26, 2019
@eladav eladav deleted the feat-get-latest-deps-versions branch September 26, 2019 08:42
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.

None yet

2 participants