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

fix: workspace script sharing #86

Merged
merged 4 commits into from
Apr 30, 2021
Merged

fix: workspace script sharing #86

merged 4 commits into from
Apr 30, 2021

Conversation

TarikGul
Copy link
Member

This PR aims to handle a behavior that yarn 2.x introduces. Being in a workspace we now need to share scripts accessing bin commands from @substrate/dev. This is only useful if you are developing in a single package and want to run the build locally.

For example, if we call yarn build from the root directory it has access to the bin commands, and will then be accessible to the script calls in each packages package.json. If we were to cd into one of the packages they no longer have access, therefore we need to share scripts from the root directory. That can be achieved by adding a colon : in the root package.json directory followed by a cd $INIT_CWD && in the scripts itself.

For example: "build:workspace": "cd $INIT_CWD && rimraf lib/ && substrate-exec-tsc -p tsconfig.build.json"

You then would just call it as yarn build:workspace in the packages scripts.

Copy link
Contributor

@emostov emostov left a comment

Choose a reason for hiding this comment

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

Thanks for the nice PR description.

Things look close:

  1. I think we can get rid of build:examples?
  2. I would like to keep the template as simple and as close to a conventional typescript project as as possible. I expect people to basically copy and paste the directory, so ideally it relies as minimally as possible on workspace specific things.

Question: what is the use case for building just package at a time? Seems like it could maybe get the inter-workspace deps confused?

packages/txwrapper-examples/package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
packages/txwrapper-template/package.json Outdated Show resolved Hide resolved
TarikGul and others added 3 commits April 29, 2021 14:43
Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
Co-authored-by: Zeke Mostov <32168567+emostov@users.noreply.github.com>
@TarikGul
Copy link
Member Author

TarikGul commented Apr 29, 2021

Awesome made those changes.

I removed build: template, and build:example, and changed template to stay as just tsc.

Question: what is the use case for building just package at a time? Seems like it could maybe get the inter-workspace deps confused?

The main idea behind it was noticed when I was working in examples, and I only needed to update and run the local code there and didnt want to re build each package. Then I thought that having workshare scripts would help mitigate any future issues with packages, but also if one wanted to work lets say in txwrapper-core without having to rebuild everything that would be nice. In terms of inter-workspace deps I think it should be fine. As long as you are only changing one package then it shouldnt be an issue.

Copy link
Contributor

@emostov emostov left a comment

Choose a reason for hiding this comment

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

🌞

@TarikGul TarikGul merged commit 207d0bb into main Apr 30, 2021
@TarikGul TarikGul deleted the tarik-fix-workspace branch April 30, 2021 12:12
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.

None yet

3 participants