Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

"unreachable code after return statement" in many packages #5518

Open
rjgotten opened this issue Dec 20, 2024 · 2 comments
Open

"unreachable code after return statement" in many packages #5518

rjgotten opened this issue Dec 20, 2024 · 2 comments

Comments

@rjgotten
Copy link

rjgotten commented Dec 20, 2024

Describe the bug; what happened?
Console in browser developer tools shows long list of warnings related to unreachable code after return statement.

Tracing through source maps:

  • @azure/communication-react/preprocess-dist/calling-component-bindings/src/handlers/createHandlers.ts:67:6
  • @azure/communication-react/preprocess-dist/react-components/src/components/ChatMessage/ChatMessageContent.tsx:152:2
  • @azure/communication-react/preprocess-dist/react-components/src/components/utils/ChatMessageComponentUtils.tsx:53:2
  • @azure/communication-react/preprocess-dist/react-components/src/components/LocalVideoTile.tsx:128:4
  • @azure/communication-react/preprocess-dist/react-components/src/components/VideoGallery/DefaultLayout.tsx:104:2
  • @azure/communication-react/preprocess-dist/react-components/src/components/VideoGallery/FloatingLocalVideoLayout.tsx:147:2
  • @azure/communication-react/preprocess-dist/react-components/src/components/VideoGallery/SpeakerVideoLayout.tsx:134:2
  • @azure/communication-react/preprocess-dist/react-components/src/components/VideoGallery.tsx:440:6
  • @azure/communication-react/preprocess-dist/calling-component-bindings/src/participantListSelector.ts:28:6
  • @azure/communication-react/preprocess-dist/react-composites/src/composites/CallComposite/components/buttons/ScreenShare.tsx:28:4
  • @azure/communication-react/preprocess-dist/react-composites/src/composites/common/CallingDialpad.tsx:78:4
  • @azure/communication-react/preprocess-dist/react-composites/src/composites/common/PeoplePaneContent.tsx:49:4
  • @azure/communication-react/preprocess-dist/react-composites/src/composites/common/VideoEffectsPane.tsx:139:2
  • @azure/communication-react/preprocess-dist/react-composites/src/composites/CallComposite/components/CallArrangement.tsx:245:4
  • @azure/communication-react/preprocess-dist/react-composites/src/composites/CallComposite/components/MediaGallery.tsx:163:6

What are the steps to reproduce the issue?
Load up the @azure/communications-react package and start integrating and using it.

What behavior did you expect?
For the code to be written in such a way that it doesn't contain useless never-executed code after unconditional return statements.

If applicable, provide screenshots:
Image

In what environment did you see the issue?

  • @azure/communication-react npm package version (if applicable): 1.22.0

  • @azure/communication-calling npm package version (if applicable): N/A (@azure/communication-react package uses bad internalized depencies from /dist-esm folder instead of actually relying on the installed @azure/communication-calling package as it claims)

  • @azure/communication-chat npm package version (if applicable): N/A (@azure/communication-react package uses bad internalized depencies from /dist-esm folder instead of actually relying on the installed @azure/communication-chat package as it claims)

  • OS & Device: Windows

  • Browser: Firefox

  • Browser Version: latest

Is there any additional information?

Another issue complicating matters is that @azure/communication-react does not actually handle its dependencies the way it should.
It installs the dependent packages; and then does not actually use them at all. Instead it uses locally bundled files built into its own dist-cjs or dist-esm folder.

So to get this fixed not only do all dependent packages need to be updated, the 'main' communication-react package does as well.
If the dependent packages are set up in the same broken way, all of them would need to be fixed and rebuilt in the proper order, for the communication-react package to finally get a fully fixed dist folder.

(In general; the entire dependency management should just be cleaned up and external dependencies should be external dependencies. I.e. should NOT be part of your own package's local dist folder.)

@alkwa-msft
Copy link
Contributor

Greetings. Thanks for this detailed summary so the engineering team has more information to look into. We will discuss internally and share what are some of our next steps soon.

@alkwa-msft
Copy link
Contributor

alkwa-msft commented Dec 20, 2024

@rjgotten

  1. For the top issue. It did highlight an unexpected product of how we are stabilizing new improves in our stable flavor and some lines were forgotten to be removed as part of this. We are setting up an internal clean up task and are looking to remove this over the coming weeks. We are excited to see if there are any performance improvements in bundle size with the reduction in this code. Thanks for your support here in calling this out.

  2. The additional information issue seems to be a separate issue. We do have a fairly complex build system to help us do internal development as well as creating a unified release which can create extra complexity when understanding the repo.

Dependency Handling: You mentioned that @azure/communication-react installs dependencies but does not use them directly, instead relying on locally bundled files. Could you specify which dependencies are affected and how this impacts your project?

Update and Rebuild Process: You indicated that all dependent packages need to be updated and rebuilt in the proper order. Could you list the specific packages involved and describe the challenges you face during this process?

Dependency Management Cleanup: You suggested that external dependencies should not be part of the local dist folder. Could you elaborate on the issues this causes and how you envision the ideal setup?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants