Skip to content

Fix lerna packages #1635

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

Merged
merged 1 commit into from
Aug 22, 2018
Merged

Fix lerna packages #1635

merged 1 commit into from
Aug 22, 2018

Conversation

raymondfeng
Copy link
Contributor

@raymondfeng raymondfeng commented Aug 22, 2018

Fix how to use lerna to collect packages.

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@raymondfeng raymondfeng requested a review from bajtos as a code owner August 22, 2018 18:45
});
const tsconfig = require(tsconfigFile);
const refs = [];
for (const d of p.localDependencies.keys()) {
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 guessing localDependencies doesn't include packages from npm?

}

const rootTsconfigFile = path.join(project.rootPath, 'tsconfig.json');
const rootTsconfig = require(path.join(project.rootPath, 'tsconfig.json'));
Copy link
Contributor

Choose a reason for hiding this comment

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

can be simplified to:
const rootTsConfig = require(rootTsConfigFile);

const rootTsconfigJson = JSON.stringify(rootTsconfig, null, 2);
if (process.argv[2] === '-f') {
// Using `-f` to overwrite tsconfig.build.json
fs.writeFileSync(rootTsconfigFile, rootTsconfigJson + '\n', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not use await writeFileAsync() ... a promisified version of this api?

@@ -24,6 +24,12 @@ function run(argv, options) {
const glob = require('glob');
const fse = require('fs-extra');

if (options === true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we even allow options to be set as a boolean?

Copy link
Contributor

@virkt25 virkt25 left a comment

Choose a reason for hiding this comment

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

Shouldn't this PR have contained the updated tsconfig files with the project references?

@raymondfeng raymondfeng force-pushed the fix-lerna-packages branch 2 times, most recently from d69f841 to 46e0772 Compare August 22, 2018 20:09
@raymondfeng
Copy link
Contributor Author

@virkt25 I spun off the TS project refs to #1636.

@raymondfeng raymondfeng merged commit ec37f2f into master Aug 22, 2018
@raymondfeng raymondfeng deleted the fix-lerna-packages branch August 22, 2018 20:30
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.

3 participants