stacks-archive / blockstack-app-generator

Blockstack app generator
MIT License
43 stars 28 forks source link

Generator for hello-blockstack now uses Blockstack@19 #43

Closed thinkverse closed 5 years ago

thinkverse commented 5 years ago

I updated the generator to use blockstack@19 instead of blockstack@18. Also fixed the issue of blockstack@19 not working with browserify by adding @babel/core and babelify.

Removed opnsince the package has been deprecated and renamed to open instead.

Updated app.js to use the new UserSession and AppConfig since the older function are getting deprecated later.

zone117x commented 5 years ago

@thinkverse Thanks for doing the v19 update work here.

The babel + browserify issue is a bug in blockstack.js, thanks for finding this. Looks like I actually introduced this bug when adding this to the blockstack.js package.json:

 "browserify": { "transform": [ [ "babelify" ] ] }

I was unaware that browserify checked dependencies for this field. It's intended use is for the blockstack.js dist build only.

The latest npm beta package ("blockstack": "19.1.0-beta.1") is using a different build system and this bug no longer exists.

Once v19.1 is released, we should update this PR to use that, and not have to add the babel related dependencies.

thinkverse commented 5 years ago

I was unaware that browserify checked dependencies for this field. It's intended use is for the blockstack.js dist build only.

@zone117x Yeah I though it was weird when updating to v19 browserify didn't work anymore and it kept asking for babelify and after that was added it asked for ©babel/core, good to see it'll be fixed later.

Also don't know what to add for the base app config for users to get a better understanding of what it does, I know some apps only add the scope, I thought about only adding that instead, but since we aren't using the scope in this example I thought it might be better just to add a basic app config instead.

Especially since there isn't really any documentation on what scopes a user can add I was thinking it might just be best to leave it en empty array for the time being.

Maybe add a README.md file with examples or tips for how a user can expand this demo further later on? Maybe with links to the react one or the vue one that I heard was in the works, thoughts? Or maybe just a link to the docs?

zone117x commented 5 years ago

I don't know what to add for the base app config for users to get a better understanding of what it does

I'm also unsure about what to do here. I think perhaps we should wait to use the v19 UserSession until we have it better documented, and have our tutorials using it.

@moxiegirl what do you think we should do here?

moxiegirl commented 5 years ago

@zone117x The dev meeting yesterday we decided to update both the generator (@hstove ) and the tutorials. I can do the tutorials (Hello World and Auth) this week (Thursday) -- @yknl is doing the Storage one. Getting the generator to 19.0.1 which is released is perhaps best as we have all the new people with the hackathons that Xan is running.

thinkverse commented 5 years ago

@moxiegirl Seems like a good idea since the functions used now are getting deprecated, it would be a good idea to keep the generators and tutorials as up to date as possible, might solve the problem other projects seem to have of users posting on forums an adding issues asking about how to do the basic setup. Might not be a problem now but you'll never know.

hstove commented 5 years ago

I think perhaps we should wait to use the v19 UserSession until we have it better documented, and have our tutorials using it.

I personally disagree - I think we should do everything with the focus on getting everything consistent and up-to-date, meaning using UserSession.

I've extended this PR in #44, just updating to v19.1.0. Thanks for doing this, @thinkverse !

thinkverse commented 5 years ago

I personally disagree - I think we should do everything with the focus on getting everything consistent and up-to-date, meaning using UserSession.

I do agree that we should stay up-to-date, what I think @zone117x meant though is that maybe we should wait with merging v19 PR's til after the tutorials and docs are using UserSession that way users won't be confused when the generator are using blockstack.UserSession.isUserSignedIn() but in the docs and getting started is says blockstack.isUserSignedIn()

The docs seem to be getting some lovin' atm so maybe it's time to update it to v19, but it's up to you Blockstack staff so.

moxiegirl commented 5 years ago

I wouldn't wait on the docs for this...we only have a couple that are using the generator. And I can fix those today.

timstackblock commented 5 years ago

Install works well when selecting the YES option. I will uninstall and select the No not ready to download and verify that as well. Was there a checklist used to verify the previous builds?

Screen Shot 2019-04-18 at 12 52 35 PM
zone117x commented 5 years ago

I approved the linked PR https://github.com/blockstack/blockstack-app-generator/pull/44 (removed the babel dependencies, and fixed the UserSession constructor usage)

@hstove @thinkverse should we merge 44 into this PR, then merge this one into dev?

thinkverse commented 5 years ago

That's up to you guys I guess.

zone117x commented 5 years ago

@thinkverse if you merge 44 into this PR we could go with this one since you initiated it.

thinkverse commented 5 years ago

Okay, that was feature/app-v19 correct? So I just need to change from master to feature/app-v19 then? First time dealing with PR's so you have to bear with me.

zone117x commented 5 years ago

I think what you want to do is merge the feature/app-v19 branch into this branch

thinkverse commented 5 years ago

44 merged to this PR.

moxiegirl commented 5 years ago

@thinkverse Thank you for the PR! Much appreciated. 💯