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

fix(stemmer): fix handling of stemmer imports #128

Merged
merged 2 commits into from Sep 7, 2022

Conversation

codyzu
Copy link
Contributor

@codyzu codyzu commented Sep 7, 2022

hotfix for #126

  • enables the TS allowJS flag so that the TS compiler copies the js files from stemmer as suggested here
  • added a "smoke" test that runs a basic search after importing lyra from dist. This is an attempt to avoid errors like this in the future. I added this test to the CI also.
  • removes the cjs build of the stemmer and renames the esm build to lib. Now that the typescript compiler handles importing, it should only be esm

Notes

  1. The npm run script ci with coverage checks isn't actually run by the CI and notably the coverage checks are failing. Probably want to fix this in the future

@codyzu
Copy link
Contributor Author

codyzu commented Sep 7, 2022

replaces and closes #127

@codyzu codyzu changed the title Fix stemmer inclusion fix(stemmer): fix handling of stemmer imports Sep 7, 2022
@codyzu
Copy link
Contributor Author

codyzu commented Sep 7, 2022

fixes #126

@ShivamJoker
Copy link

I am facing the same problem can this be merged soon?

@micheleriva
Copy link
Member

We'll get this fixed by this evening

@codyzu
Copy link
Contributor Author

codyzu commented Sep 7, 2022

If you're blocked and don't want to roll back to the previous release or wait until tonight for a new release, you can try out my fix by removing Lyra from your package.json and then npm install "codyzu/lyra#fix-stemmer-inclusion"

@micheleriva
Copy link
Member

Thank you Cody, I'm gonna merge this

@micheleriva micheleriva merged commit 19b5120 into askorama:main Sep 7, 2022
@micheleriva
Copy link
Member

@ShivamJoker this is fixed

@ShivamJoker
Copy link

Awesome I'll upgrade and let you know

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

3 participants