Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: bootstrap as an option #735

Merged
merged 1 commit into from
Jan 29, 2017
Merged

Conversation

@daviddias daviddias added the status/in-progress In progress label Jan 29, 2017
@daviddias daviddias force-pushed the feat/enable-bootstrap-by-env branch from e46a643 to 721a6ca Compare January 29, 2017 08:45
@daviddias daviddias force-pushed the feat/enable-bootstrap-by-env branch from 721a6ca to c2871c8 Compare January 29, 2017 09:06
@victorb
Copy link
Member

victorb commented Jan 29, 2017

I'm a bit worried about using process.env in the browser bundle, with risk of confusing people. Historically, browser javascript has not cared about environment variables, my only known example of environment variables in the wild is React which uses NODE_ENV to determine if it should do the type checking of props and other things, only if it's not set to production.

In webpack it's relatively simple to inject process.env, and without bundler you could simply create a global object before loading the script. I think we just have to make sure we document how you can change them in both cases, with bundler and without.

@daviddias
Copy link
Member Author

Understood, as noted in IRC:

16:46 <@victorbjelkholm> daviddias: ah, that's cool
16:47 <@victorbjelkholm> daviddias: process.env in the browser bundle, not sure about that, thinking we maybe should try to figure out an alternative way
16:47 <@daviddias> suggestions?
16:47 <@victorbjelkholm> experimental config object as part of the config or the options?
16:48 <@victorbjelkholm> not sure really but thinking it's gonna confuse people
16:49 <@daviddias> The thing with that is that it creates the pattern of passing down special config options everywhere for experimental things
16:51 <@victorbjelkholm> yeah, and surely that's bad. Depends on what we think is worse, passing down config options as feature flags, or environment variable which browsers doesn't really used but can be hacked in by creating global variables
16:52 <@daviddias> think I got the bug, somewhere we are using a old version of the cid package
16:52 <@daviddias> victorbjelkholm: I believe that `IPFS_BOOTSTRAP=1 npm run build` will pick it up
16:52 <@daviddias> and since most browser apps are always being bundled
16:52 <@daviddias> it means it always picks up that

Note that this is intended to be a quick way to try out Bootstrap, until we have a full compatible muxer, after that it should be always on.

Nevertheless, we should figure out a way to have experimental features that are enabled by some sort of 'env variables' everywhere.

@daviddias
Copy link
Member Author

For exploration - #738

@daviddias daviddias merged commit 03362a3 into master Jan 29, 2017
@daviddias daviddias deleted the feat/enable-bootstrap-by-env branch January 29, 2017 17:54
@daviddias daviddias removed the status/in-progress In progress label Jan 29, 2017
@daviddias daviddias mentioned this pull request Feb 1, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants