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

Bundle library using rollup #198

Merged
merged 7 commits into from
Apr 10, 2022
Merged

Bundle library using rollup #198

merged 7 commits into from
Apr 10, 2022

Conversation

lukasIO
Copy link
Contributor

@lukasIO lukasIO commented Apr 8, 2022

Initial motivation was #129 and that subsequently lead to

  • use rollup as a bundler
  • create a umd and esm bundle
  • follow best practices for package.json entry files
  • use babel (instead of tsc) for transpilation in order to target browser compatibility more specifically
  • remove webpack and replace with rollup dev server for /example

some resources/projects used as inspiration:

The rollup build process started to show some warning around our protobuf.js dependency. Specifically some circular dependencies within that library and the frowned-upon usage of eval within the library.
Rollup seems to deal well with the circular dependencies, to workaround the usage of eval there are some annotations/additional context in rollup.config.js where basically every instance of eval gets replaced with undefined.

@lukasIO lukasIO requested a review from davidzhao April 8, 2022 13:07
@lukasIO lukasIO marked this pull request as ready for review April 8, 2022 13:07
@lukasIO lukasIO requested a review from davidliu April 8, 2022 13:35
"build-docs": "typedoc",
"proto": "protoc --plugin=node_modules/ts-proto/protoc-gen-ts_proto --ts_proto_opt=esModuleInterop=true --ts_proto_out=./src/proto --ts_proto_opt=outputClientImpl=false,useOptionals=true -I./protocol ./protocol/livekit_rtc.proto ./protocol/livekit_models.proto",
"sample": "cd example && webpack serve",
"build-sample": "cd example && webpack && cp styles.css index.html dist/",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the build-sample command (without dist present it was failing for me) but can re-add with a rollup equivalent if that command still makes sense/we want to continue to provide it

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

This is really fantastic. I'm reading about rollups and have learned something new today!

Copy link
Contributor

@davidliu davidliu left a comment

Choose a reason for hiding this comment

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

So, something with rollup is screwing up react-native. Can't figure out what it is, but the javascript engine it's using doesn't like it. When debugging (which uses the computer's browser javascript engine), it works again.

For now, since react-native is still in alpha, I think we'll lock down the livekit-client versions that it can use until we figure out a more permanent solution.

@lukasIO lukasIO merged commit d43bea5 into main Apr 10, 2022
@lukasIO lukasIO deleted the lukas/add-bundles branch April 10, 2022 17:50
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.

3 participants