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: compiler no longer throwing an error #292

Merged
merged 3 commits into from
Nov 17, 2017
Merged

fix: compiler no longer throwing an error #292

merged 3 commits into from
Nov 17, 2017

Conversation

crowmagnumb
Copy link
Contributor

I'm submitting a...

[x] Bug Fix
[ ] Feature
[ ] Other (Refactoring, Added tests, Documentation, ...)

Checklist

  • Commit Messages follow the Conventional Commits pattern
    • A feature commit message is prefixed "feat:"
    • A bugfix commit message is prefixed "fix:"
  • Tests for the changes have been added

Description

please describe the changes that you are making

complier-cil.performCompilation was now silently failing resulting in no compiled code getting written and so when the source mapper ran it got an error because the file it was trying to map didn't exist. Again because performCompilation was silently failing. This checks the result of the compilation to see if it resulted in an error and tries to give the best info it can about this and then throws an exception to stop the whole packaging process. Notice I had problems getting the typescript namespace (ts) recognized in the ng-packagr build process so I had to comment that out.

for features, please describe how to use the new feature

please include a reference to an existing issue, if applicable

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@dherges
Copy link
Contributor

dherges commented Nov 17, 2017

Hi @crowmagnumb,

thanks for the PR!

Please bare with me and be a little patient, there is change coming in #279

performCompilation(compilerConfig);
const compilerResult = performCompilation(compilerConfig);

if (!compilerResult.emitResult) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Luckily, there are helper functions for this from @angular/compiler-cli.

  const exitCode = ng.exitCodeFromResult(result.diagnostics);
  if (exitCode === 0) {
    return Promise.resolve(
      path.resolve(ngPkg.sourcePath, tsConfig.options.outDir, tsConfig.options.flatModuleOutFile)
    );
  } else {
    return Promise.reject(new Error(
      ng.formatDiagnostics(result.diagnostics)
    ));
  }

If you like an early fix, I ask you to make a PR with these changes. Otherwise, let's wait for #279. I just need a few more days to correct the "write staged files to dist".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I'm not in a super hurry on that front but I did make the changes you suggested and it's much better so I just added them here. :)

@dherges
Copy link
Contributor

dherges commented Nov 17, 2017

Travis is not running for this PR?!? Why the heck that is... don't know ... Circle is green, and the change looks good to me, so let's go for it!

@dherges dherges merged commit 815509b into ng-packagr:master Nov 17, 2017
@dherges
Copy link
Contributor

dherges commented Nov 17, 2017

@crowmagnumb Is now published in 2.0.0-rc.0 / ng-packagr@next

https://yarnpkg.com/en/package/ng-packagr

@github-actions
Copy link

This PR has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

This action has been performed automatically by a bot.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants