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

Add generator search through Algolia yarn index #763

Merged
merged 3 commits into from
Apr 20, 2018

Conversation

pixelastic
Copy link
Contributor

Hello,

Tim from Algolia here. I wanted to suggest this PR that updates the current generator search to provide a search experience similar to the one you can have on the Yarn website.

2018-04-13_11h24m22
You can try it yourself right here.

This actually uses the exact same dataset as the one used on Yarn (containing all the npm packages), but filtered to only show yeoman generators. Results are ranked based on the popularity (number of downloads and dependents). All the data is coming directly from npm, and is updated in real time whenever a package is added/deleted/edited. Others (like Gatsby, Babel or CodeSandbox) are also relying on the same index.

It is slightly different than the current implementation, but it has a few advantages: it is faster and handles highlighting and fuzzy matching out of the box. I'd be happy to discuss the implementation further (there is still room for improvement regarding the display on mobile), but I first wanted to check your interest in such a feature.

@mischah mischah requested review from SBoudrias, zckrs and a team April 13, 2018 18:29
Copy link
Member

@mischah mischah left a comment

Choose a reason for hiding this comment

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

Hej Tim,

I love it. Thanks for the PR 💖

Our package which is creating the current generator list was completely rewritten a year ago but still has some hickups from time to time. Hope this will be rock solid (stable) solution.

I just wonder if it would be possible to blacklist a few modules like we did before. Must say that I’m not sure about the importance of that blacklisting feature because the blacklist wasn’t updated for quite a while.

Cheers, Michael

P.S. Greetings to Luky and Haroen 😘

@pixelastic
Copy link
Contributor Author

Glad you like it!

The list currently only return packages that have yeoman-generator as part of the keywords. It does not (yet) respect the fact that the package should also start with generator-. I've submitted a patch for the yarn index about that and that should be available soon. I'll update the PR when it will be ready.

I think this solution might be more stable as you'll not be the only one relying on it, and we actively monitor it for any issue.

The disallowed list being pretty short, I think a quick fix would be to not render results that are matching an element of that list directly from the front-end.

P.S: Your love has been passed to them :)

@SBoudrias
Copy link
Member

Awesome PR thanks a lot for sending it :)

I think the ranking algorithm will remove the need for the blacklist. I'm happy to drop it personally.

apiKey: '14ddf54fd4f3435c1cd4038395a0cf10',
indexName: 'npm-search',
searchParameters: {
filters: 'deprecated:false AND keywords:yeoman-generator'
Copy link
Contributor

Choose a reason for hiding this comment

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

filters: 'deprecated:false AND computedKeywords:yeoman-generator'

I changed the Yarn index to now have a subset of the complete registry, they need the keyword and start with generator-. You can see (and edit if necessary) the code for that here

Don't hesitate to reach out if there's any questions about this

@mischah
Copy link
Member

mischah commented Apr 19, 2018

I guess it won’t be possible to highlight the generators maintained by the Yeoman team like we used to do?

image

@SBoudrias I’m not sure how important that is since a lot of them are pretty much unmaintained 🙈

@Haroenv
Copy link
Contributor

Haroenv commented Apr 19, 2018

How can you recognise if it's maintained by the Yeoman team, owner.name?

var name = highlight(item, 'name').replace('generator-', '')
var url = getUrl(item);
var description = highlight(item, 'description');
var authorName = item.owner.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

var authorName = item.owner.name === "yeoman" ? "The Yeoman Team" : item.owner.name;

that would do what you suggested @mischah, could even add a custom class or whatever if needed

@mischah
Copy link
Member

mischah commented Apr 19, 2018

It was stored in a offical property over here:
https://github.com/yeoman/yeoman-generator-list/blob/3536bf0c8f0ba47b28a1f00bf8850982108c16b4/src/npms-package-info.js#L60

Hold on. I think this is obsolete because your solution makes it christal clear the a generator is from the team:

image

@Haroenv
Copy link
Contributor

Haroenv commented Apr 19, 2018

yeah, already clear enough as-is :)

@SBoudrias
Copy link
Member

This change looks great to me :)

@mischah I'm good to have this merged in if you are!

I'm wondering if we could also update the yo cli search to use algolia?

@Haroenv
Copy link
Contributor

Haroenv commented Apr 20, 2018

Should be possible, but it's not that easy making an interactive cli. How are you fetching the search now?

@Haroenv
Copy link
Contributor

Haroenv commented Apr 20, 2018

just be sure that the change I suggested in #763 (comment) is addressed before merging :)

@mischah
Copy link
Member

mischah commented Apr 20, 2018

@SBoudrias

@mischah I'm good to have this merged in if you are!

I just want to check the possibilities to improve the experience on small viewports first.

@Haroenv

How are you fetching the search now?

Via https://github.com/sindresorhus/npm-keyword over here:
https://github.com/yeoman/yo/blob/479250a28220a6dcb75130f687f2e097498d4cfc/lib/routes/install.js#L44

This will only include the packages starting with `generator-`
Copy link
Contributor

@Haroenv Haroenv left a comment

Choose a reason for hiding this comment

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

looks good as expected

Copy link
Member

@mischah mischah left a comment

Choose a reason for hiding this comment

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

Thanks to everyone involved ♥️

@mischah mischah merged commit dd31965 into yeoman:source Apr 20, 2018
@mischah
Copy link
Member

mischah commented Apr 20, 2018

It’s live and it’s looking good 🎉

Thanks, @pixelastic @Haroenv 💝

@SBoudrias I’m going to schedule a tweet from the Yeoman account for Monday to gain more attention.

@DirtyF
Copy link
Contributor

DirtyF commented Apr 20, 2018

Thanks @pixelastic 😉

@mischah
Copy link
Member

mischah commented Apr 21, 2018

Hej @SBoudrias, one last note about this change:

I’ve just closed all existing issues of yeoman-generator-list and removed the travis cron job.

Transformation completed.

completed

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

5 participants