-
Notifications
You must be signed in to change notification settings - Fork 67
chore: upgrade ipfs to 0.55.x #661
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
Conversation
Do I have to create a fleek account to see why CI fails?
@achingbrain I'm afraid so.
I checked and it's failing on lesson 7 of the Regular Files API tutorial.
Ah, right:
return bufferedContents.toString()
probably needs to change to:
return new TextDecoder().decode(bufferedContents)
Updates to the latest ipfs release. Some extra build config was needed because it uses optional chaining. This is supported out of the box by the version of acorn used by webpack 5 but vue is currently using webpack 4 so some extra config was necessary. Also uprades `to-buffer` - this now returns `Uint8Array`s instead of the node `Buffer` built in, which means you can't call `.toString(encoding)` on it any more, instead you need to use the global js `TextDecoder` class.
074fc9e to
a23125f
Compare
@terichadbourne ready for your review.
I'm not sure about the description of the new TextDecoder().decode(), but you can read through it and see if it's ok.
Thank you @achingbrain for working on this! 🙏
Thank you both for all of your help with this. I just went through each lesson and compared the output of the solution code to the lesson contents about what the method is supposed to return. There were a few methods where this had changed, eg as pointed out in #691), so I updated the lesson text to match the output I actually see.
@achingbrain Could I ask you to do me the favor of reviewing my 4 recent commits (all small) to ensure you agree with the way I've described what the user should now expect? In the last commit, there were a few cases where I didn't know how to properly describe what a field meant (or the unit it was presented in and would love your help. (I add TODOs in the code.)
The last TODO in the Regular Files API tutorial is a spot where we tell them that the result of ipfs.get will include a field content that contains a Buffer but only if the type is file. What I actually see in the results suggests that when it's a file, it returns an empty object, and when it's a directory it omits the field.
Thanks in advance for your help on this!
@achingbrain Just a ping here - could you find time to review my last 4 commits and make sure they're accurate and answer the question in my comment above? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field will be present if found in the DAG so it's probably worth keeping it here with a note that it's an optional property.
That is, mode gets a default value depending on if the entry is a file or directory, but mtime does not.
The last TODO in the Regular Files API tutorial is a spot where we tell them that the result of ipfs.get will include..
It's probably worth revisiting this exercise. In ipfs@0.57.0 the output of ipfs.get changed from sort of being a bit like a recursive ipfs.ls to outputting tarballs in line with the CLI/go-IPFS HTTP API.
ipfs.get also had a fairly significant caveat in that over HTTP the API has always outputted a tarball which was then unpacked by the client to present the same API as core, which means that if you were using the HTTP API client you'd have to consume the contents of a file before you could move on to the next entry, which is a terribly leaky abstraction and not a nice thing to have to explain to people.
There's now a clear reason to use ipfs.get vs ipfs.ls as the former exports a UnixFS file/directory structure in an archive and the latter lists metadata about a file/directory and would be used in conjunction with ipfs.cat to get file contents and ipfs.ls to descend further into the directory structure.
Updates to the latest ipfs release. Some extra build config was
needed because it uses optional chaining.
This is supported out of the box by the version of acorn used by
webpack 5 but vue is currently using webpack 4 so some extra config
was necessary.
Also uprades
to-buffer- this now returnsUint8Arrays insteadof the node
Bufferbuilt in, which means you can't call.toString(encoding)on it any more, instead you need to use theglobal js
TextDecoderclass.