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

build(bazel): allow a user to control a subset of angularCompilerOption in their tsconfig file when using ng_module #28995

Conversation

gregmagolan
Copy link
Contributor

Unblocks #28689 which requires the use of

  "angularCompilerOptions": {
    "i18nInFile": "translations.xlf"
  }

…on in their tsconfig file when using ng_module
@alexeagle alexeagle added the target: patch This PR is targeted for the next patch release label Feb 26, 2019
@gregmagolan gregmagolan added the action: merge The PR is ready for merge by the caretaker label Feb 26, 2019
@benlesh benlesh added area: build & ci Related the build and CI infrastructure of the project area: bazel Issues related to the published `@angular/bazel` build rules labels Feb 27, 2019
@ngbot ngbot bot modified the milestone: needsTriage Feb 27, 2019
// Allow Bazel users to control some of the bazel options.
// Since TypeScript's "extends" mechanism applies only to "compilerOptions"
// we have to repeat some of their logic to get the user's "angularCompilerOptions".
if (config['extends']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah in my case with this code it gets: /home/ocombe/.cache/bazel/_bazel_ocombe/df516b31b5c55b244dfb17f03a952d3d/execroot/angular/packages/tsconfig-build.json but it doesn't read the local tsconfig file that I defined in my bundle test folder


// All user angularCompilerOptions values that a user has control
// over should be collected here
if (userConfig.angularCompilerOptions) {
Copy link
Contributor

@ocombe ocombe Feb 27, 2019

Choose a reason for hiding this comment

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

Can't you make a loop that gets all properties of userConfig? We wouldn't need to maintain this when we add new compiler config options. And worst case scenario it would set an option that wouldn't be used

Copy link
Contributor

Choose a reason for hiding this comment

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

there are some properties which bazel needs to control, I think flatModuleId is an example.

ngc-wrapped is planned to be deprecated with Ivy so we don't need to put that much care here about maintainability

benlesh pushed a commit that referenced this pull request Feb 27, 2019
…on in their tsconfig file when using ng_module (#28995)

PR Close #28995
benlesh pushed a commit that referenced this pull request Feb 27, 2019
@benlesh benlesh closed this in c4c3c12 Feb 27, 2019
benlesh pushed a commit that referenced this pull request Feb 27, 2019
@ocombe
Copy link
Contributor

ocombe commented Feb 27, 2019

@benlesh I don't think it was ready for merge, @alan-agius4 seems to have a legitimate comment, and the PR doesn't seem to fix my issue (which was the goal)
it's probably not your fault since there was a merge label :(
I guess there should be a follow up PR then

@gregmagolan
Copy link
Contributor Author

@benlesh I don't think it was ready for merge, @alan-agius4 seems to have a legitimate comment, and the PR doesn't seem to fix my issue (which was the goal)
it's probably not your fault since there was a merge label :(
I guess there should be a follow up PR then

@ocombe I tried the failure target with this change and it fixed it locally. Are you still failing on the same issue?

@eachirei
Copy link

Sorry to hop in, but couldn't this also include

{
    "skipTemplateCodegen": true,
    "strictMetadataEmit": true,
    "fullTemplateTypeCheck": true
  }

I got some issues when building my project regarding

Error during template compile of 'AppModule'
  Function calls are not supported in decorators but 'StoreModule' was called.

and I understand that these options could fix that issue.

@gregmagolan
Copy link
Contributor Author

@eachirei fullTemplateTypeCheck should default to true (you can also control that one with the type_check attribute on ng_module. The other two may be controllable under Bazel but I'm not sure of the implications of controlling metadata emit under Bazel which is why I left them out from the PR.

Have you looked at #23609?

@eachirei
Copy link

I'll give it another shot and try the method described in latest comment on that issue.

@eachirei
Copy link

Thing is the modules that are causing this problem are from external libraries, so I'll try to wrap them up with that method. I do not know the real cause of this, but I don't think this approach is feasible though.

@angular-automatic-lock-bot
Copy link

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

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: bazel Issues related to the published `@angular/bazel` build rules area: build & ci Related the build and CI infrastructure of the project cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants