jspm / generator

JSPM Import Map Generator
Apache License 2.0
165 stars 21 forks source link

chore: update ipfs #187

Closed Aslemammad closed 1 year ago

Aslemammad commented 1 year ago

Resolves #186

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

guybedford commented 1 year ago

Looks like the tests just need updating:

+ 'https://ga.jspm.io/npm:lit@2.4.1/index.js'
- 'https://ga.jspm.io/npm:lit@2.3.1/index.js'
    at file:///home/runner/work/generator/generator/test/api/update.test.js:54:10 {

Also, please sign the digital CLA per the comment.

For IPFS testing, we likely will need to uncomment that section in the workflow, it seems I removed it in the Chomp PR https://github.com/jspm/generator/pull/101.

Aslemammad commented 1 year ago

Ipfs works properly now. Both in my system, and also I added it in the CI.

guybedford commented 1 year ago

Thanks, that's great to see! Ran it locally and that seems to be the case.

The Skypack test was disabled because it wasn't working (see https://github.com/jspm/generator/issues/185).

Hopefully we can trace the remaining CI issues as well.

guybedford commented 1 year ago

I was able to resolve the skypack, port collision and firefox CI failures here, so we are closer to green. I believe this error in the CI is actually an IPFS error:

TypeError: Class extends value undefined is not a constructor or null
    at Object.<anonymous> (/home/runner/work/generator/generator/node_modules/timeout-abort-controller/index.js:8:33)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at ModuleWrap.<anonymous> (internal/modules/esm/translators.js:199:29)
    at ModuleJob.run (internal/modules/esm/module_job.js:183:25)
    at async Loader.import (internal/modules/esm/loader.js:178:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5)
    at async handleMainPromise (internal/modules/run_main.js:59:12)

Since this dependency is only being introduced by IPFS.

Also I think the fact that I had to change the server port to get the tests to pass from 8080 means that the IPFS server might be running on that port instead, when IPFS should I think be running on its own port. Perhaps you can look into this further.

Aslemammad commented 1 year ago

Hey Guy, sure, I'm going to work on it.

Aslemammad commented 1 year ago

@guybedford Green now.

Aslemammad commented 1 year ago

@guybedford How can we resolve the lit issue in the ci?

guybedford commented 1 year ago

What's the CI issue?

Aslemammad commented 1 year ago

Oh, i see the ci is updated now, ok, nothing for now.

guybedford commented 1 year ago

The tests need to be passing to merge, let me know if there's anything I can do to help.

Aslemammad commented 1 year ago

Green now.