This is very interesting, I think that in the future I’ll refactor Kit to use this to get User data, will probably make everything faster!
This seems like a good opportunity for the npm package to include both the existing webpacked browser dist file as well as the regular non-processed source that can provide nice code-completion / intellisense / docs in editors like VSCode. It also allows developers to specify their own packing for nodejs or web apps.
Attempting to import * as blockstack from 'blockstack/lib/index' doesn’t help since the ES6 modules in blockstack/src/*.js are destroyed when compiled to the blockstack/lib dir.
Documentation pages is great, but intellisense while coding against libs is much more convenient.
One approach would be performing the Flow transpile process without babel, then use babel/webpack to create the dist file. This wouldn’t change the existing workflow for those directly importing the dist for browser usage, and would allow importing from blockstack/lib/index for intellisense. I used a similar setup when playing around with porting blockstack.js from Flow to Typescript and it seemed to work well.
This would also fix another minor issue with the current setup where all (?) the package.json/dependencies should really be package.json/devDependencies.
This should probably be posted as a separate issue - just thinking if there have to be two breaking changes might as well be in the same release. Thoughts?
Example of VSCode unable to resolve anything useful from blockstack lib while coding:
1 Like
Good to know. Is there a migration path we’re recommending, or do we still need to write one?
I think this is a really great change, and brings about lots of benefits that have been discussed here.
I’m not super happy about making such a large breaking change, though. We already have issues where many apps aren’t using the most up-to-date version of blockstack.js, which can lead to issues during onboarding and elsewhere. In those cases, upgrading blockstack.js doesn’t even involve breaking changes.
With this change, apps will have to refactor their entire app. If those apps didn’t upgrade before, they surely won’t now, and existing apps will have to set aside some dev work to upgrade.
I don’t see why we can’t keep this as a non-breaking upgrade. We could still export all of the existing top-level metrics and have them use a UserSession object underneath. Then, all existing code would still work.
It could be something like this:
// blockstack.js top-level `getFile`
const getFile = (*args) => {
const session = new UserSession();
return session.getFile(*args);
}
I’d really not like to break existing web apps, because the vast majority of blockstack apps are web apps, and thus have no use for this new change. It seems like we’re making a big breaking change, for a very small amount of existing developers.
4 Likes