Skip to content
This repository was archived by the owner on Jul 10, 2025. It is now read-only.

Conversation

coder11
Copy link
Contributor

@coder11 coder11 commented Feb 2, 2023

@linear
Copy link

linear bot commented Feb 2, 2023

DXJ-26 Migration to libp2p v0.37.0

libp2p v0.37.0 switched to ESM instead of CommonJS.

A decision is needed in order to process with migration

  • Migrate to ESM?
  • Leave libp2p v0.36.2?
  • Transpile ESM to CommonJS?
  • Other options???

Release checklist:

  • Bump libp2p version in forked repo
  • Optimize unit/integration tests layour
  • Release sub-packages
  • Test version against examples
  • Configure CI

@coder11 coder11 requested a review from shamsartem February 3, 2023 12:55
@linear
Copy link

linear bot commented Feb 3, 2023

DXJ-223 Reuse prettier.rc config across packages

It's better to create a package with it and reuse it everywhere inside pacakge.json instead of copypasting (it's really easy - just needs an entry in pcakages,json like this prettier: "@fluencelabs/prettierrc").

DXJ-214 Shared tsconfig file in JS Client packages

DXJ-195 Implement Web JS Client bundling

We take Web distro for JS client and with the help of webpack\rollup\etc bundle the code into the single *.min.js bundle

.npmrc Outdated
@@ -1,2 +1,3 @@
auto-install-peers=true
save-exact=true
@fluencelabs:registry=https://npm.fluence.dev/
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this necessary? shouldn't it work with the default npm registry?

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 are waiting for the fix from machines team. Once it's get done we will switch to release version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now!

@@ -0,0 +1,19 @@
{
Copy link
Contributor

@shamsartem shamsartem Feb 3, 2023

Choose a reason for hiding this comment

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

Is package-lock.json necessary top-level? Because there is already a pnpm-lock.yaml here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, package-lock.json isn't necessary at any level since we are using pnpm. The idea behind pacakge.json is to extract commonly used dev dependencies into a single place

// @ts-ignore
globalThis.defaultPeer = makeDefaultPeer();

// TODO! remove
Copy link
Contributor

Choose a reason for hiding this comment

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

please do remove, or if this hack is still necessary - please add a task and link it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the todo with more detailed info

// export const avmModuleLoader = new WasmNpmLoader('@fluencelabs/avm', 'avm.wasm');

export const controlModuleLoader = new WasmLoaderFromFs(
'/home/pavel/work/fluence/fluence-js/node_modules/.pnpm/@[email protected]/node_modules/@fluencelabs/marine-js/dist/marine-js.wasm',
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not contain your local absolute path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

);

export const avmModuleLoader = new WasmLoaderFromFs(
'/home/pavel/work/fluence/fluence-js/node_modules/.pnpm/@[email protected]/node_modules/@fluencelabs/avm/dist/avm.wasm',
Copy link
Contributor

Choose a reason for hiding this comment

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

this too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed as well

Copy link
Contributor

@shamsartem shamsartem left a comment

Choose a reason for hiding this comment

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

Please checkout my minor comments. Thanks for the job!

@coder11 coder11 requested a review from shamsartem February 6, 2023 08:30
Copy link
Contributor

@shamsartem shamsartem left a comment

Choose a reason for hiding this comment

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

Thanks! Hope the stuff gets finished in the tasks liked to TODOs

@coder11 coder11 merged commit a1265f4 into before-denver Feb 6, 2023
@coder11 coder11 deleted the DXJ-26-update-libp2p branch February 6, 2023 12:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants