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 module field to package.json #333

Merged
merged 1 commit into from
Oct 2, 2022
Merged

Conversation

benmccann
Copy link
Contributor

No description provided.

@ts-thomas ts-thomas merged commit 1298807 into nextapps-de:master Oct 2, 2022
@benmccann benmccann deleted the patch-1 branch October 2, 2022 17:07
@justin0mcateer
Copy link

For me, this causes an problem with Rollup (via Vite) recognizing the types. It claims none of the types are exported.

@hanneskuettner
Copy link

hanneskuettner commented Feb 28, 2023

@justin0mcateer Same problem for me. This also seems to cause the problems in #341.

This is, as far as I can see, due to a misunderstanding. dist/module/index.js is not actually the entrypoint for this project but contains only the Index class (the entrypoint seems to be webpack.js). So setting "module" to this results in a broken import for projects that rely on ESM.

The easy solution would be to revert for now. Then ESM projects will fall back to the normal bundle. This won't allow any tree-shaking as far as I'm aware, but that seems like a battle for another day, probably using package entry points. This would mean creating a proper entrypoint.js, which is different from the overarching FlexSearch export in the webpack.js to align with the types provided in @typings/flexsearch.

@ts-thomas I've confirmed locally that removing the line fixes any issues with ESM on my end. Could we roll this back as it seems to be working perfectly in the parent commit? Do you want a PR for this?

@benmccann Could you clarify why you made this change and if it worked for you? Cause this definitely seems like it should not work.

@benmccann
Copy link
Contributor Author

I am using flexsearch 0.7.31 with Vite in my project and it appears to be working, but it's definitely possible that I screwed something up if it's broken for others. Honestly, I find this project's build process to be quite arcane. It's very difficult to tell what's in dist/module/index.js since it's minified. Usually npm modules are distributed in unminified form and the end-user should be responsible for bundling their dependencies together and minifying at that time. It also looks like this project is built using Google Closure Compiler, which is a pretty uncommon choice. I think folks would better able to contribute to the project if a more common build tool like Rollup were used or better yet if the source were just distributed without any type of build process since it's already ESM.

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.

4 participants