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

Move to esbuild #79

Merged
merged 2 commits into from
Oct 12, 2023
Merged

Conversation

kevinwhereby
Copy link
Contributor

@kevinwhereby kevinwhereby commented Oct 10, 2023

Description

We are having some funny module resolution problems with rollup, so this pr aims to replace it with esbuild

Adds build steps for individual react/embed builds as well

Gotchas

Esbuild can't emit typescript declarations, so we have to do it with tsc
Unfortunately, tsc can't generate es6 concatenated declarations so we include the full directory structure... is this a problem?

To Test

  • yarn to install the stuff
  • yarn build to generate the builds and types
  • import it into a demo project

Screenshots

Here's me importing just the react bundle like import { useRoomConnection } from "@whereby.com/browser-sdk/react"
image

and using the embed bundle in a script tag like
<script type="module" src="./node_modules/@whereby.com/browser-sdk/dist/embed/index.esm.js"></script>
image

@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch 3 times, most recently from 2bb70ec to 6b44ebb Compare October 10, 2023 23:04
tsconfig.build.json Outdated Show resolved Hide resolved
@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from 6b44ebb to bca9773 Compare October 11, 2023 07:15
@kevinwhereby
Copy link
Contributor Author

Hmm it seems the fullly bundled embed build doesn't work...
image

@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from bca9773 to 343a8f8 Compare October 11, 2023 11:57
@kevinwhereby
Copy link
Contributor Author

@havardholvik got the full embed bundle working, I think we're good to go now

@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from 343a8f8 to 38a70b2 Compare October 11, 2023 11:58
@havardholvik
Copy link
Collaborator

@havardholvik got the full embed bundle working, I think we're good to go now

Nice! Im doing some review and testing now, will be done shortly

@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from 38a70b2 to 583dddd Compare October 12, 2023 08:29
roomUrl: string,
roomConnectionOptions: UseRoomConnectionOptions
): RoomConnectionRef {
export function useRoomConnection(roomUrl: string, roomConnectionOptions: UseRoomConnectionOptions): RoomConnectionRef {
Copy link
Contributor Author

@kevinwhereby kevinwhereby Oct 12, 2023

Choose a reason for hiding this comment

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

This default export was causing webpack build errors when importing into a Create React App app.

@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from 07f72d2 to 60577be Compare October 12, 2023 14:37
@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from 60577be to f3bbd6a Compare October 12, 2023 14:38
@kevinwhereby
Copy link
Contributor Author

kevinwhereby commented Oct 12, 2023

The react build isn't isolated because for some reason even though the entry point for that builds is specified as src/lib/react/index.ts, the root src/lib/index.ts is being evaluated and included in the build, which imports both modules... unsure how to fix this.

esbuild doesn't support generating type declarations like rollup doesn

This is a little problematic, as now we have a separate build step that
tells tsc to generate our types.

tsc doesn't support generating a single index.d.ts file for es6 modules,
so we emit our directory structure.
@kevinwhereby kevinwhereby force-pushed the kevinhanna/pan-329-fix-assert-issue branch from f3bbd6a to 106772b Compare October 12, 2023 16:18
@kevinwhereby kevinwhereby merged commit 586c7d5 into development Oct 12, 2023
1 check passed
@kevinwhereby kevinwhereby deleted the kevinhanna/pan-329-fix-assert-issue branch October 12, 2023 21:34
@kevinwhereby kevinwhereby restored the kevinhanna/pan-329-fix-assert-issue branch October 12, 2023 21:34
This was referenced Oct 12, 2023
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