-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Resolve local module linking in example apps #1286
Comments
@tmeasday Thanks so much for investigating this. I don't have great ideas on how to move forward. In the short term, I am sidelining my RN-related example app changes and pushing forward with the react-only stuff. What a colossal pain in the ... neck. |
As suggested by npm/npm#15900. This is a workaround until we get a better resolution of #1286
As suggested by npm/npm#15900. This is a workaround until we get a better resolution of #1286
Note that we have gone with technique B. in #1332, but I'll leave this open for a longer-term solution. |
Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks! |
I think this problem is mostly OK now. |
Not really. When testing RN, you have to do |
@danielduan does the bot ignore mentions from PRs? This issue had one 3 days ago |
@tmeasday I was working on implementing a new logger in RN, but the process of building and testing with the example was indeed a nightmare. I have some stuff I was working (haul and workspaces) for the RN example over the weekend, but probably won't have time to get to until the new year. I can push my (broken) code if you would like to take a look and/or a crack at it. |
Oh! My bad, sorry! |
And @dangreenisrael may as well post any information / branches to this issue as you can, hopefully someone can figure it out! |
Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks! |
Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook! |
With the switch to npm@5, the way that the
test-cra
andreact-native-vanilla
apps were linked (viafile://
links) changed from "installs" to symlinks, similarly to lerna's behavior. See npm/npm#15900 for more info.To start with, this sort of removes the point of
test-cra
as it behaves the same ascra-storybook|kitchen-sink
.However, there are some problems with symlinking our local dependencies that the
file://
links solved. In particular:Jest ends up executing modules multiple times (Jest requires symlinked files twice jestjs/jest#3830), leading to issues of this form, or this form.
RN cannot resolve over symlinks ([Packager] doesn't resolve modules that are symlinks facebook/react-native#637) making our example apps non-functional, and mostly pointless.
I'm not sure if/when the above issues will be solved, so we may want to consider other solutions to the problem. Here are some things that we can investigate:
A. Only supporting npm<5 for development.
B. Following the recommended fallback of
npm pack
-ing our packages and creatingfile://
links to the.tar.gz
(or using a tool like https://www.npmjs.com/package/install-local to do that).C. Investigating yarn's support for workspaces -https://github.com/yarnpkg/rfcs/blob/master/implemented/0000-workspaces-install-phase-1.md
I would appreciate thoughts on all this from anyone with experience with this kind of thing.
The text was updated successfully, but these errors were encountered: