Skip to content

Conversation

@Lezek123
Copy link
Contributor

@Lezek123 Lezek123 commented May 25, 2020

This PR is an attempt to move Pioneer from the repository at https://github.com/Joystream/apps into this monorepo.

Moving Pioneer into subfolder and merging

I created a monorepo-migration branch in the Pioneer repository (https://github.com/Joystream/apps/tree/monorepo-migration), where I moved all the files into pioneer folder. This branch was then merged into pioneer-migraton (this one) using --allow-unrelated-histories

The entire flow was like this:

cd LOCAL_APPS_ROOT
git checkout joystream
git branch monorepo-migration
git checkout monorepo-migration
mkdir pioneer
ls -a1 | grep -v ^pioneer | xargs -I{} git mv {} pioneer
git commit -m "Move to subfolder"
git push upstream monorepo-migration

cd LOCAL_MONOREPO_ROOT
git checkout master
git remote add pioneer-upstream https://github.com/Joystream/apps.git
git fetch pioneer-upstream
git branch pioneer-migration
git checkout pioneer-migration
git merge pioneer-upstream/pioneer-migration --allow-unrelated-histories

The 3k+ commits in this PR are the result of moving the entire Pioneer history. All the configuration work described below is introducted in the last commit of this PR: 643c08c

Workspaces

In order to have all current Pioneer's workspaces available among other monorepo workspaces, I landed on this configuration in the root directory packages.json:

	"workspaces": [
		"tests/network-tests",
		"cli",
		"types",
		"pioneer",
		"pioneer/packages/*"
	],

As you can see, I made a few adjustments there, ie.:

  • Moved joy-types package from the Pioneer's packages directory into the root monorepo directory types. I think it doesn't make much sense to have joy-types nested inside Pioneer anymore. It is a quite universal library, which is used by many projects, like the CLI, query node, Atlas (?) etc. therefore I think it makes sense for it to be at least at the same level in the directory structure as all those other projects. It allso allows as to consume it inside Pioneer the same way it is consumed by other projects (ie. we have to specify @joystream/types/lib as an import, instead of @joystream/types.

  • I removed the nested workspaces in pioneer/packages.json as I figured we don't need to define them twice if they are already defined in root package.json via pioneer/packages/*. It seems more clear and less error-prone to have all workspaces defined only once (in the root folder).

  • Defined cli as one of the workspaces in order to test the effect of using shared @joystream/types among Pioneer and CLI.

The idea behid this structure is that we can run yarn install in the root directory in order to install and link dependencies for all the workspaces at once and then execute any command in Pioneer workspace ie. by running:

yarn workspace pioneer COMMAND
Where COMMAND can be ie.: build, start, storybook etc.

Because Pioneer was very dependent on beeing a monorepo itself (instead of beeing part of a larger monorepo), this turned out not to be possible before making a few adjustments described below.

Yarn, webpack and other issues

In order to make Pioneer work as part of a larger monorepo (with commands like start, build, storybook and lint remaining usable) I needed to:

1 Fix the issue with yarn.lock beeing ignored

If Pioneer's lockfile is removed/ignored, yarn installs some conflicting versions of packages like multicodec (v0.5 and v1.0), which causes Pioneer's start script to fail. When Pioneer becomes just a workspace in a monerepo, it's yarn.lock is no longer respected unless moved into the root directory. This forced me to include yarn.lock in the root folder as part of the commit.

It's probably not good that yarn.lock is required in order for the Pioneer to work, so we might want to investigate this issue further, this isn't strictly related to the process of moving into the monorepo though (it happens if we remove yarn.lock in the original repository too) and there were a lot of other issues that needed fixing as well, so I decided not to tackle this one now.

  1. Pioneer doesn't build with newer TypeScript (ie. 3.8.3)

If we don't specify the exact version of typescript in the Pioneer's package.json, we might get some newer version due to hoisting, which causes Pioneer build to fail. I fixed it by specifying "typescript": "3.7.2" in Pioneer's package.json (this is the version it was using when installed from it's own repository).

  1. Some sub-dependency binaries not beeing linked in the workspace's node_modules

Binaries from some sub-dependency packages like webpack, @babel/cli etc. are not beeing linked in the pioneer/node_modules/.bin after running yarn install in the root directory unless those packages are specified as direct dependencies of Pioneer. This required adding packages like: @babel/cli, @polkadot/dev, webpack. cpx to Pioneer's packages.json (I tried using the same versions that would normally get installed).

  1. Eslint plugins need to be required in Pioneer's package.json (cannot be found when hoisted)

There was some issue with eslint not beeing able to find plugins (which were sub-dependencies of ie. @polkadot/dev-react) hoisted to root node_modules also, so those needed to be specified as direct dev dependencies in Pioneer's package.json too. Those include: eslint-config-semistandard, eslint-config-standard, eslint-plugin-import, eslint-plugin-node, eslint-plugin-promise and eslint-plugin-standard

  1. Pioneer's tsconfig looking for files in ./node_modules

Another issue was that due to relative path to some node_modules in Pioneer's tsconfig.json, some files could not be found by the TypeScript compiler. Those paths needed to be changed from ./node_modules to ../node_modules.

  1. @joystream/types vs @joystream/types/lib

Because @joystream/types was moved to separate workspace, independent from Pioneer, the imports from @joystream/types needed to be replaced to imports from @joystream/types/lib. This was also mentioned in: #321

I fixed it by executing following replace-by-regex's for all project files:

Replace from '@joystream/types(?!/lib) to from '@joystream/types/lib
Replace from "@joystream/types(?!/lib) to from "@joystream/types/lib
Replace require\('@joystream/types(?!/lib) to require('@joystream/types/lib

Strangely enough I noticed that, for example, the cli was succesfully using import from @joystream/types (without lib) before, even though it was always consuming @joystream/types externally. Perhaps it's due to main beeing specified in types/package.json. This may require some further investigating in order to make sure it's resolved correctly.

  1. The need for .yarnclean

I moved the .yarnclean file from pioneer folder into root folder.
I also needed to add @polkadot/ts/node_modules there, because for some reason Yarn was creating node_modules folder inside @polkadot/ts, which was causing TypeSciprt error and a failure while trying to build Pioneer.

Current .yarnclean looks as follows:

@types/react-native
@polkadot/ts/node_modules
  1. "types": ["node"] in @joystream/types tsconfig

There was a need to add "types": ["node"] in types/tsconfig.json. Otherwise some type conflicts between jest and mocha were preventing yarn workspace @joystream/types build from running succesfully.

  1. pioneer/.storybook/webpack.config.js - needed to add ".js" extension

There was an error while trying to run yarn workspace pioneer storybook probably due to the fact that @joystream/types workspace was moved outside Pioneer and webpack wans't configured to process .js files that were coming from there.

How to test

While testing this branch I was so far mostly focused on those 3 things:

  1. Do Pioneer commands run succesfully? (ie. build, start, storybook, lint)
  2. Can I share @joystream/types workspace between Pioneer and CLI?
  3. Does both CLI and Pioneer work correctly?

In order to initialize the monorepo (before trying to test anything) there are 2 steps required:

  1. Run yarn install in the root directory (remove all node_modules before that if needed).
  2. Run yarn workspace @joystream/types build (in order to build the types, so that the other workspaces can succesfully import them).

After that, commands like this should execute succesfully:

  • yarn workspace pioneer start
  • yarn workspace pioneer build
  • yarn workspace pioneer storybook
  • yarn workspace pioneer lint

In order to try CLI commands, you can run them like this (from the root directory):
./cli/bin/run council:info

Gamaranto and others added 30 commits April 21, 2020 11:10
…mint

Proposal form: Content Working Group Mint Capacity (from MintCapacityForm)
add missing types to @joystream/types
some variables got a bit stronger typing. The build errors went down to
18 from 40.
@Lezek123 Lezek123 marked this pull request as draft May 25, 2020 14:37
@Lezek123 Lezek123 requested a review from mnaamani May 25, 2020 14:37
@mnaamani
Copy link
Member

Brilliant work on this migration.

I hope when/if we pull from upstream polkadot-js/apps merge conflicts will not be too difficult to deal with, and I'm sure documenting the changes after import here will come in handy.

The only issue I have is that @joystream/types still transpiles into the lib/ folder and the import path used by consuming apps still have to contain this /lib. I always felt like I made a bad choice with that approach and was hoping to make it more uniform with all the other packages in pioneer where code is transpiled into build/ and somehow when importing the "build/" is not required in the path. Is this some webpack magic or different typescript/babel configuration done for other packages?

Can we improve on joystream/types in a similar way?

When working in old Pioneer repo, one didn't need to think about having to build dependencies before the main build, it was automatic.

Here as you pointed out we have to remember to rebuild @joystream/types and then the project that depends on it. I'm guess webpack was always taking care of rebuilding dependencies so you could just modify types and run yarn start and code would be automatically transpiled.

Is there a way to get a similar behavior in this new repo. What I'm imagining is that after someone clones the repo, they just need to run yarn and it will install and build all dependencies including any local ones and build all our apps, so the next step is just to run one of the apps.

I see you are using "*" as the version for the @joystream/types package in the cli pacakge.json will this not be a problem when/if we choose to publish the cli for example to npm registry.

We should remove pioneer/yarn.lock and cli/yarn.lock now that only the top .lock file is required.

I don't quite understand the resolutions section in the package.json but doesn't the one in pioneer/package.json need to be moved to the root package.json? It does seem to be relevant to any app that is depending on the polkadot/api to ensure they all use the same versions.

@mnaamani
Copy link
Member

I was able to build pioneer and also run it with development yarn workspace pioneer start works well.

Just wondering why is @joystream/types added in devDependencies and not just dependencies ?

@Lezek123
Copy link
Contributor Author

Lezek123 commented May 27, 2020

I wasn't able to make it all work the way we would want for now, but at least manged to make the workflow with Pioneer as simple as it was before, while maintaining types in the root folder and easily accessible for other projects.

I configured pioneer/packages/apps/webpack.config.js and pioneer/tsconfig.js so that now they are aware of the new location of @joystream/types and all the changes are processed on-fly when running yarn workspace pioneer start (same way it was functioning before the merge into monorepo), so we don't need to "manually" build @joystream/types anymore.

Optimally we would want something like that for the CLI (and potentially other projects) too, but I couldn't find a way to make it work for now, so the CLI and tests/network-tests still rely on the build version of @joystream/types.

Having that in mind I added yarn workspace @joystream/types build as a postinstall script in the root package.json, so at least we can work on those tools right after running yarn install. We just need to remember to re-build the @joystream/types if we decide to change them along the way.

One more thing to keep in mind is that this gets us back to the disparity of how @joystream/types are "consumed" by Pioneer vs other projects. The other projects still need to add the /lib if importing from other files/folders than @joystream/types/lib/index.js.

I think this may be a good-enough setup for now, it could definitely be improved, but that would require some more research on my part and we'd have to probably change the way we're handling the process of building and publishing @joystream/types etc.

I also resolved a few of the smaller issues mentioned:

  • @joystream/types beeing a devDependency by mistake
  • Using "*" for the version of @joystream/types
  • Moved the "resolutions" section from Pioneer to root package.json

@Lezek123 Lezek123 marked this pull request as ready for review May 27, 2020 12:13
@mnaamani mnaamani merged commit 560cbac into Joystream:development May 28, 2020
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.

6 participants