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

[chore]: improve configure-agora-sdk.js #1444

Closed
BlackHole1 opened this issue Mar 28, 2022 · 2 comments
Closed

[chore]: improve configure-agora-sdk.js #1444

BlackHole1 opened this issue Mar 28, 2022 · 2 comments
Assignees
Labels
stale Stale issues or PR

Comments

@BlackHole1
Copy link
Collaborator

@crimx

Originally posted by @BlackHole1 in #1442 (comment)

@hyrious
Copy link
Member

hyrious commented Mar 28, 2022

FYI, one of the purposes of configure-agora-sdk.js is updating the .npmrc file before yarn install, so that agora-electron-sdk will read correct values before it downloads the binary.

However, this behavior is not needed after using pnpm's neverBuiltDependencies. The downloading work is totally handed to the main-app/postinstall script.

The node-linker=hoisted option was used to address some badly developped storybook plugins. But that will cause the other problem -- common dependencies in different versions will be hard to locate the correct version. In case you haven't seen it, the dependency find-up has 4 versions in our project:

> pnpm why -r find-up@2
devDependencies:
eslint-config-react-app 7.0.0
└─┬ eslint-plugin-import 2.25.4
  └─┬ eslint-module-utils 2.7.3
    └── find-up 2.1.0

> pnpm why -r find-up@3
...@storybook...
webpack 4.46.0
   ...
   find-up 3.0.0

> pnpm why -r find-up@4
@storybook/semver
└── find-up 4.1.0

> pnpm why -r find-up@5
@storybook/manager-webpack5 6.3.1
   ...
   find-up 5.0.0

So how could that option break things? It is because when you have multiple different versions of the same package, pnpm will create symlinks to different folders (all hoisted to the top), like such:

find-up // default version
find-up@4.1.0
...
some-package
  |- node_modules
       |- find-up ~> find-up@4.1.0

If some package's custom resolver (like vite) did not follow symlinks, it will find the wrong dependency, which may break things.

@stale
Copy link

stale bot commented May 27, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale issues or PR label May 27, 2022
@stale stale bot closed this as completed Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issues or PR
Projects
None yet
Development

No branches or pull requests

3 participants