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

feat: add helia-car-creator example #72

Merged
merged 11 commits into from
Aug 1, 2023
Merged

feat: add helia-car-creator example #72

merged 11 commits into from
Aug 1, 2023

Conversation

SgtPooki
Copy link
Contributor

@SgtPooki SgtPooki commented Jun 14, 2023

NOTE: This example was created to assist some folks in ETHGlobal HackFS 2023 who were asking questions about how to build CAR files out of files uploaded by a user.

Things left to do:

  • Update test
  • Update README to explain what this example is for

@SgtPooki SgtPooki marked this pull request as draft June 14, 2023 06:16
@SgtPooki SgtPooki changed the title feat: add helia-car-creator example [WIP] feat: add helia-car-creator example Jun 14, 2023
@SgtPooki SgtPooki changed the title [WIP] feat: add helia-car-creator example feat: add helia-car-creator example Jul 28, 2023
@SgtPooki SgtPooki marked this pull request as ready for review July 28, 2023 20:36
@SgtPooki
Copy link
Contributor Author

failing test shows different car file CID, see https://github.com/ipfs-examples/helia-examples/actions/runs/5696092971/job/15440516463?pr=72#step:6:135

  1. test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file
Error: expect(received).toContain(expected) // indexOf

Expected substring: "bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry"
Received string:    "bafybeiczsscdsbs7ffqz55asqdf3smv6klcw3gofszvwlyarci47bgf354"

  57 |     const cidOutputContent = await page.textContent(cidOutput)
  58 |
> 59 |     expect(cidOutputContent).toContain(expectedCarCID)
     |                              ^
  60 |
  61 |     // download the car file
  62 |     const [download] = await Promise.all([

    at /home/runner/work/helia-examples/helia-examples/examples/helia-create-car/test/index.spec.js:59:30

https://cid.ipfs.tech/#bafybeiczsscdsbs7ffqz55asqdf3smv6klcw3gofszvwlyarci47bgf354 shows

base32 - cidv1 - dag-pb - (sha2-256 : 256 : 59948439065F29619EF41280CBB932BE52C56D99C5966B65E0111239F098BBEF)

https://cid.ipfs.tech/#bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry shows

base32 - cidv1 - dag-pb - (sha2-256 : 256 : B2536C9B3838F998D82F29D6FD11439F875F252A8CFCAB22564E1F41FE08178E)

@SgtPooki
Copy link
Contributor Author

there is definitely something sketchy going on somewhere.

assuming something with playwright downloads

╰─ ✘ 1 ❯ hyperfine 'npm run test' --runs=25 --show-output                                                           9.23   24.5G   40%   100%  ─╯
Benchmark 1: npm run test

> helia-create-car@0.0.0 test
> npm run build && test-browser-example test


> helia-create-car@0.0.0 build
> vite build

vite v4.4.7 building for production...
../../node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "../../node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
✓ 953 modules transformed.
dist/index.html                     0.46 kB │ gzip:   0.31 kB
dist/assets/index-db66ab3e.css      1.40 kB │ gzip:   0.73 kB
dist/assets/index-69166d97.js   1,485.83 kB │ gzip: 439.55 kB

(!) Some chunks are larger than 500 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 3.01s

Running 1 test using 1 worker

  ✓  1 test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file (1.0s)
roots [ CID(bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry) ]
roots[0].toString():  bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry

  1 passed (1.7s)

> helia-create-car@0.0.0 test
> npm run build && test-browser-example test


> helia-create-car@0.0.0 build
> vite build

vite v4.4.7 building for production...
../../node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "../../node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
✓ 953 modules transformed.
dist/index.html                     0.46 kB │ gzip:   0.31 kB
dist/assets/index-db66ab3e.css      1.40 kB │ gzip:   0.73 kB
dist/assets/index-69166d97.js   1,485.83 kB │ gzip: 439.55 kB

(!) Some chunks are larger than 500 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 3.04s

Running 1 test using 1 worker

  ✓  1 test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file (1.7s)
roots [ CID(bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry) ]
roots[0].toString():  bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry

  1 passed (2.4s)

> helia-create-car@0.0.0 test
> npm run build && test-browser-example test


> helia-create-car@0.0.0 build
> vite build

vite v4.4.7 building for production...
../../node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "../../node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
✓ 953 modules transformed.
dist/index.html                     0.46 kB │ gzip:   0.31 kB
dist/assets/index-db66ab3e.css      1.40 kB │ gzip:   0.73 kB
dist/assets/index-69166d97.js   1,485.83 kB │ gzip: 439.55 kB

(!) Some chunks are larger than 500 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 3.24s

Running 1 test using 1 worker

  ✓  1 test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file (1.7s)
roots [ CID(bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry) ]
roots[0].toString():  bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry

  1 passed (2.4s)

> helia-create-car@0.0.0 test
> npm run build && test-browser-example test


> helia-create-car@0.0.0 build
> vite build

vite v4.4.7 building for production...
../../node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "../../node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
✓ 953 modules transformed.
dist/index.html                     0.46 kB │ gzip:   0.31 kB
dist/assets/index-db66ab3e.css      1.40 kB │ gzip:   0.73 kB
dist/assets/index-69166d97.js   1,485.83 kB │ gzip: 439.55 kB

(!) Some chunks are larger than 500 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 2.95s

Running 1 test using 1 worker

  ✓  1 test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file (2.0s)
roots [ CID(bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry) ]
roots[0].toString():  bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry

  1 passed (2.8s)

> helia-create-car@0.0.0 test
> npm run build && test-browser-example test


> helia-create-car@0.0.0 build
> vite build

vite v4.4.7 building for production...
../../node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "../../node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
✓ 953 modules transformed.
dist/index.html                     0.46 kB │ gzip:   0.31 kB
dist/assets/index-db66ab3e.css      1.40 kB │ gzip:   0.73 kB
dist/assets/index-69166d97.js   1,485.83 kB │ gzip: 439.55 kB

(!) Some chunks are larger than 500 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 3.32s

Running 1 test using 1 worker

  ✓  1 test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file (1.2s)
roots [ CID(bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry) ]
roots[0].toString():  bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry

  1 passed (1.9s)

> helia-create-car@0.0.0 test
> npm run build && test-browser-example test


> helia-create-car@0.0.0 build
> vite build

vite v4.4.7 building for production...
../../node_modules/@protobufjs/inquire/index.js (12:18) Use of eval in "../../node_modules/@protobufjs/inquire/index.js" is strongly discouraged as it poses security risks and may cause issues with minification.
✓ 953 modules transformed.
dist/index.html                     0.46 kB │ gzip:   0.31 kB
dist/assets/index-db66ab3e.css      1.40 kB │ gzip:   0.73 kB
dist/assets/index-69166d97.js   1,485.83 kB │ gzip: 439.55 kB

(!) Some chunks are larger than 500 kBs after minification. Consider:
- Using dynamic import() to code-split the application
- Use build.rollupOptions.output.manualChunks to improve chunking: https://rollupjs.org/configuration-options/#output-manualchunks
- Adjust chunk size limit for this warning via build.chunkSizeWarningLimit.
✓ built in 3.10s

Running 1 test using 1 worker

  ✘  1 test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file (779ms)


  1) test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file 

    Error: expect(received).toContain(expected) // indexOf

    Expected substring: "bafybeifsknwjwoby7gmnqlzj236rcq47q5pskkum7svsevsod5a74caxry"
    Received string:    "bafybeiczsscdsbs7ffqz55asqdf3smv6klcw3gofszvwlyarci47bgf354"

      57 |     const cidOutputContent = await page.textContent(cidOutput)
      58 |
    > 59 |     expect(cidOutputContent).toContain(expectedCarCID)
         |                              ^
      60 |
      61 |     // download the car file
      62 |     const [download] = await Promise.all([

        at /Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/examples/helia-create-car/test/index.spec.js:59:30

  1 failed
    test/index.spec.js:43:3 › Use Helia With react and vite › files can be converted to a valid car file 
file:///Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/node_modules/execa/lib/error.js:60
                error = new Error(message);
                        ^

Error: Command failed with exit code 1: npx playwright test test
    at makeError (file:///Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/node_modules/execa/lib/error.js:60:11)
    at handlePromise (file:///Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/node_modules/execa/index.js:124:26)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async file:///Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/node_modules/test-ipfs-example/bin/test-browser-example.js:12:1 {
  shortMessage: 'Command failed with exit code 1: npx playwright test test',
  command: 'npx playwright test test',
  escapedCommand: 'npx playwright test test',
  exitCode: 1,
  signal: undefined,
  signalDescription: undefined,
  stdout: undefined,
  stderr: undefined,
  cwd: '/Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/examples/helia-create-car',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

Node.js v20.5.0
npm ERR! Lifecycle script `test` failed with error: 
npm ERR! Error: command failed 
npm ERR!   in workspace: helia-create-car@0.0.0 
npm ERR!   at location: /Users/sgtpooki/code/work/protocol.ai/ipfs-examples/helia-examples/examples/helia-create-car 
Error: Command terminated with non-zero exit code: 1. Use the '-i'/'--ignore-failure' option if you want to ignore this. Alternatively, use the '--show-output' option to debug what went wrong. 

@SgtPooki
Copy link
Contributor Author

made some changes, and then ran

> hyperfine 'npm run test' --show-output --ignore-failure --export-json=./testRuns.json --runs=1000 | tee testRuns.txt
[...]
> cat testRuns.json | jq '.results[0].exit_codes | {total: . | length, failed: map(select(. == 0|not)) | length }'
{
  "total": 1000,
  "failed": 0
}

Copy link
Contributor

@whizzzkid whizzzkid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job, works great on my local! Approving in principle, some minor nits for your consideration.

Comment on lines +4 to +8
npm-debug.log*
yarn-debug.log*
yarn-error.log*
pnpm-debug.log*
lerna-debug.log*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*.log*?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk why these were all there. probably from the other example i copied from

examples/helia-create-car/README.md Outdated Show resolved Hide resolved
examples/helia-create-car/index.html Outdated Show resolved Hide resolved
const downloadEl = document.createElement('a')
const blobUrl = window.URL.createObjectURL(carBlob)
downloadEl.href = blobUrl
downloadEl.download = 'test.car'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just call it the <original_file_name>.car?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could (or first filename if multiple), but I think that's a minor thing anyone running/improving this example could do easily. will sleep on this one


const { writer, out } = await CarWriter.create(rootCID)

// don't await yet..
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not?

return car(helia)
}, [helia])
const heliaFs = useMemo(() => {
if (helia == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can/Should this be DRYer?

export default function FileUploader () {
const { files, setFiles } = useFiles()
const handleFileEvent = useCallback((e) => {
const filesToUpload = Array.prototype.slice.call(e.target.files)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally find this very hacky, isn't e.target.files already an array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not quite an array but I haven't looked at this part of the code in a while, I will look again

Comment on lines +28 to +46
if (helia) {
console.info('helia already started')
} else if (window.helia) {
console.info('found a windowed instance of helia, populating ...')
setHelia(window.helia)
setFs(unixfs(helia))
setStarting(false)
} else {
try {
console.info('Starting Helia')
const helia = await createHelia()
setHelia(helia)
setFs(unixfs(helia))
setStarting(false)
} catch (e) {
console.error(e)
setError(true)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (helia) {
console.info('helia already started')
} else if (window.helia) {
console.info('found a windowed instance of helia, populating ...')
setHelia(window.helia)
setFs(unixfs(helia))
setStarting(false)
} else {
try {
console.info('Starting Helia')
const helia = await createHelia()
setHelia(helia)
setFs(unixfs(helia))
setStarting(false)
} catch (e) {
console.error(e)
setError(true)
}
}
if (helia) {
console.info('helia already started')
return
}
try {
const heliaInstance = window.helia || await createHelia()
setHelia(heliaInstance)
setFs(unixfs(helia))
setStarting(false)
} catch (e) {
console.error(e)
setError(true)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was copied from helia-vite IIRC, will test this update tomorrow

export const HeliaProvider = ({ children }) => {
const [helia, setHelia] = useState(null)
const [fs, setFs] = useState(null)
const [starting, setStarting] = useState(true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing the value of this, can't just access to the instance help?

line-height: 1.5;
font-weight: 400;

color-scheme: light dark;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my 👁️ 👁️ thank you!

SgtPooki and others added 2 commits July 30, 2023 21:52
Co-authored-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
Co-authored-by: Nishant Arora <1895906+whizzzkid@users.noreply.github.com>
@SgtPooki SgtPooki merged commit 1599420 into main Aug 1, 2023
13 checks passed
@SgtPooki SgtPooki deleted the feat/helia-create-car branch August 1, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants