Skip to content

fix: put all ngc files into a single directory#10486

Merged
mhevery merged 1 commit into
angular:masterfrom
mhevery:option2
Aug 4, 2016
Merged

fix: put all ngc files into a single directory#10486
mhevery merged 1 commit into
angular:masterfrom
mhevery:option2

Conversation

@mhevery

@mhevery mhevery commented Aug 3, 2016

Copy link
Copy Markdown
Contributor

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x")

[X] Feature

Does this PR introduce a breaking change? (check one with "x")

[X] No

Other information:

Prior to this change ngc would place generated files which refer
to components in the node_modules into the node_module. This is an
issue. Now all of the files are forced into a single directory
as specified in tsconfig.json by the genDir option.

see: https://docs.google.com/document/d/1OgP1RIpZ-lWUc4113J3w13HTDcW-1-0o7TuGz0tGx0g

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

revert

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

reverted

@IgorMinar IgorMinar added pr_state: LGTM action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The above comment no longer seem applicable as this no longer calls resolve().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

@naomiblack

Copy link
Copy Markdown
Contributor

@vikerman @alxhub -- FYI that misko will merge this one into master as soon as it's ready (possibly before the router fix from victor) so we can test against a master build in the morning. impact on internal tests should be nil.

Prior to this change `ngc` would place generated files which refer
to components in the node_modules into the node_module. This is an
issue. Now all of the files are forced into a single directory
as specified in `tsconfig.json` by the `genDir` option.

see: https://docs.google.com/document/d/1OgP1RIpZ-lWUc4113J3w13HTDcW-1-0o7TuGz0tGx0g
@mhevery

mhevery commented Aug 4, 2016

Copy link
Copy Markdown
Contributor Author

Tests added

@mhevery mhevery merged commit 790362e into angular:master Aug 4, 2016
} else {
if (!this.isGenDirChildOfRootDir) {
// assume that they are on top of each other.
importedFile = importedFile.replace(this.options.basePath, this.options.genDir);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this change is broken - we used to support pathMapping by trying candidate paths through the moduleResolver, but now we don't. it breaks google3. @vikerman

alexeagle added a commit to alexeagle/angular that referenced this pull request Aug 9, 2016
alxhub pushed a commit that referenced this pull request Aug 9, 2016
@mhevery mhevery deleted the option2 branch June 2, 2017 17:06
@angular-automatic-lock-bot

Copy link
Copy Markdown

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 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants