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

Offline mode #373

Closed
gaearon opened this issue Aug 5, 2016 · 28 comments
Closed

Offline mode #373

gaearon opened this issue Aug 5, 2016 · 28 comments
Milestone

Comments

@gaearon
Copy link
Contributor

gaearon commented Aug 5, 2016

npm wizards, we need your help!

Per @pselle’s suggestion, we should try to find a way to make create-react-app work offline.

I’m not well versed in npm flags and likely won’t have time for this, so this is a perfect opportunity for you to contribute and significantly improve the user experience of Create React App.

Here is the behavior we’re looking for:

  • Existing requirement: create-react-app my-app should use the most recent version of react-scripts if you are online. We are not changing this: we always want the users to get the most recent one.
  • New requirement: create-react-app my-app should not crash offline if you ran it at least once before. It should be able to use locally cached react-scripts in this case, if it exists.
  • Bonus: create-react-app my-app should use cached react-scripts if it knows the latest npm version is cached locally. In other words, even if you are online, we shouldn’t just refetch the whole thing if we have it locally and we know it is the latest version. This is not required but would make a big difference for most people.
  • Acceptable tradeoff: we are fine with not installing the latest transitive dependencies. In other words it’s fine if react-scripts depends on some-package@1.0.0, then some-package@1.0.1 comes out, but we keep using the cached some-package@1.0.0.

How to achieve this? I have no idea.

Maybe this would be helpful: https://addyosmani.com/blog/using-npm-offline/.

We already use bundledDependencies (which is a bit unusual) as part of our build process so in reality react-scripts is being downloaded as a single tarball rather than through the usual npm process. Perhaps this can give us the leverage to make this better (e.g. by caching that tarball somehow).

@apaleslimghost
Copy link
Contributor

I'm not sure how this fits with bundledDependencies but in npm >= 3.9 --cache-min=999999999 is pretty much exactly what you're looking for https://github.com/npm/npm/releases/tag/v3.9.0

@gaearon
Copy link
Contributor Author

gaearon commented Aug 5, 2016

I’m concerned about issues like this: npm/npm#8581. If the solution involves --cache-min, and --cache-min causes weird crashes in some conditions, we must:

  1. Understand why this happens and whether our users will be affected by this
  2. Be resistant to such failures (e.g. by retrying npm install without that flag if it fails)

If you know how to handle these points, I’m happy to review a PR explaining your thought process.

@apaleslimghost
Copy link
Contributor

I believe that issue is exactly what 3.9 fixes. You could detect npm version and avoid the flag if running older than 3.9? I'd be happy to do a pr, is this the right call to modify?

@gaearon
Copy link
Contributor Author

gaearon commented Aug 5, 2016

Yes, this is the right call. What you suggest seems reasonable, please go for it!

It would also help if you could help me understand how to reproduce exactly that issue in < 3.9 so we can verify 3.9 indeed fixes it in our case.

@apaleslimghost
Copy link
Contributor

apaleslimghost commented Aug 5, 2016

To reproduce you need a package a that depends on b@^1.1.0, where a is not in the cache, and b@1.0.0 is in the cache but b@1.1.0 is not. Then try to install a with ---cache-min=99999999. Instead of downloading the new version of b, npm will only try versions from the cache, and fail because there is no compatible version.

(Hope that helps, it's not a trivial issue...)

@gaearon
Copy link
Contributor Author

gaearon commented Aug 5, 2016

Yeah, I meant this in the context of react-scripts.

If you submitted a PR, I’d appreciate step-by-step instructions to reproduce this exact issue with react-scripts just so I can verify that it was broken before 3.9 but fixed in it.

@apaleslimghost
Copy link
Contributor

Oh, right, sorry. The user would have to have an old version of one of the dependencies of react-scripts in cache but not react-scripts itself. So if they had e.g. babel-core@6.10 in cache and tried to run npm install --cache-min=Infinity react-scripts (which depends on babel-core@6.11) it would hit the issue.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 5, 2016

Thanks for explanation! I’ll try reproing that if you submit a PR. 😉

@apaleslimghost
Copy link
Contributor

Just double checking I'm editing global-cli/index.js? That's a pretty scary warning 😉

@gaearon
Copy link
Contributor Author

gaearon commented Aug 5, 2016

Yea, it’s meant to prevent accidental breaking changes since people almost never have to update the CLI, and since old CLIs must always work with new react-scripts even after breaking changes in react-scripts. Your change is safe tho.

@gafemoyano
Copy link
Contributor

Did anyone get to this issue? Does create-react-app work properly offline?

@gaearon
Copy link
Contributor Author

gaearon commented Sep 24, 2016

No but if you pick up #375 and make it pass Travis, it could be in next version :-)

@gaearon gaearon added this to the 1.0.0 milestone Sep 24, 2016
@gafemoyano
Copy link
Contributor

gafemoyano commented Sep 24, 2016

I could give it a try, seems like the other guy has been busy. Is it a matter of making travis tests pass? Does this commit meet all the functional requirements?

@gaearon
Copy link
Contributor Author

gaearon commented Sep 24, 2016

I don’t think it meets #375 (comment).

@lukyth
Copy link
Contributor

lukyth commented Oct 11, 2016

Are you doing this, @gafemoyano? If not, I'd like to try to resolve this as well.

@gaearon
Copy link
Contributor Author

gaearon commented Oct 11, 2016

We might have another solution to this soon.

@gafemoyano
Copy link
Contributor

Sorry I haven't been able to figure it out cause... well I don't have reliable internet access. For now I've just been keeping a clean project generated with the latest version and copy/pastying it when I need to start a new app. I know it's not really a solution to this, but it's good enough considering that upgrading between CRA versions hasn't been difficult at all.

@gaearon
Copy link
Contributor Author

gaearon commented Oct 11, 2016

Okay, check it out.
We should integrate with Yarn if it exists in the system:

https://github.com/yarnpkg/yarn
https://yarnpkg.com/
https://code.facebook.com/posts/1840075619545360

It supports offline mode.

@gafemoyano
Copy link
Contributor

Oh wow this came out literally 45 minutes ago, congrats!!
So will yarn be bundled with CRA? What happens if Yarn isn't in the system, should we still try to use npm's cache?

@gaearon
Copy link
Contributor Author

gaearon commented Oct 11, 2016

Yarn won't be bundled with CRA. We should change the CLI to detect if Yarn is present. If it is, we should install with Yarn, and offline mode will "just work". If not, we'll fall back to npm with current behavior.

@gafemoyano
Copy link
Contributor

Cool, I'll read up about Yarn and report back if I can handle the integration, unless @lukyth wants to get started right away.

@lukyth
Copy link
Contributor

lukyth commented Oct 11, 2016

@gafemoyano It's fine. I just thought if you didn't want to do this anymore, I'd love to give it a try as well. 😄

@gaearon
Copy link
Contributor Author

gaearon commented Oct 11, 2016

Please comment in #896 if you work on this.

@chiptus
Copy link

chiptus commented Jan 3, 2017

yarn supports offline mode only when the flag --offline is added, so we need to check if yarn failed and if it did, install with the flag. Probably should check the error to see exactly what failed.

@chiptus
Copy link

chiptus commented Feb 22, 2017

Not sure what the status on this or on #896,i would love to work on this if possible

@Timer
Copy link
Contributor

Timer commented Feb 22, 2017

@chiptus since that issue is closed, I believe any discussion can be held here. Just please post a comment in #896 that you want to work on this and link back to this issue. We love contributions!

I would make sure that #1423 isn't already doing what this goes to accomplish; you can try to coordinate with @voxsim.

@chiptus
Copy link

chiptus commented Feb 28, 2017

@Timer it seems like #1423 is exactly this. I guess I will look for another issue to contribute to :)

@gaearon
Copy link
Contributor Author

gaearon commented Feb 28, 2017

Yea, #1423 just fixed this!
Will be out in the next version which we plan to publish today.

@gaearon gaearon closed this as completed Feb 28, 2017
@gaearon gaearon modified the milestones: 0.9.3, 1.0.0 Feb 28, 2017
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants