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(package): add proper support for webpack 4 which is used by expo #120

Merged
merged 2 commits into from
Sep 5, 2022

Conversation

codyzu
Copy link
Contributor

@codyzu codyzu commented Sep 5, 2022

  • Expo uses webpack 4 for web builds. They have a draft PR to upgrade to webpack 5: [webpack] Upgrade to Webpack 5 expo/expo-cli#3763
  • webpack 4 uses Acorn 6.x for parsing, which does not support the latest ECMA script features (such as optional chaining). webpack decided to integrate Acorn >7.3.0 only into webpack 5: Unexpected token error at code with optional chaining when using Typescript webpack/webpack#10227
  • The above 2 points mean that until expo updates to webpack 5, expo web builds does not support language features such as optional chaining ?. that are used by Lyra.
  • Solution A (this PR): fix Lyra by adding a new typescript build for "browser" (which is used by webpack 4 and webpack 5) that targets ES2019 (the latest compatible with Acorn 6.x). It will also be used by (non expo) react native projects via the metro loader. Since next.js uses webpack 5, it will likely pick up the "browser" entry for web builds too. Vite uses the "module" entry and won't be affected. Same for node.
  • Solution B (not implemented): Change the existing cjs and esm Lyra builds to target ES2019. I think we prefer "pure", non transpiled, ECMAScript when possible.
  • Solution C (bad devex): Force application builders to transpile Lyra manually when building with expo. I don't think this is reasonable because expo is a "batteries included" RN development framework.
  • once [webpack] Upgrade to Webpack 5 expo/expo-cli#3763 is merged, we can undo this special build
  • I added concurrently to build the now 3 builds in parallel

package.json Outdated Show resolved Hide resolved
tsconfig.webpack.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@micheleriva
Copy link
Member

Thanks for the PR @codyzu !

I'd go with solution A, with a specific build for react-native. Hopefully, we can get rid of it once it supports webpack 5

@codyzu
Copy link
Contributor Author

codyzu commented Sep 5, 2022

We can create an open issue the tracks the PR in expo... that way we don't forget. I'll do that now.

@micheleriva
Copy link
Member

thanks! lgtm

@micheleriva micheleriva merged commit c87ca88 into oramasearch:main Sep 5, 2022
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.

2 participants