Skip to content

feat(build): check circular depencies in Node.js#980

Closed
marclaval wants to merge 1 commit into
angular:masterfrom
marclaval:madge
Closed

feat(build): check circular depencies in Node.js#980
marclaval wants to merge 1 commit into
angular:masterfrom
marclaval:madge

Conversation

@marclaval
Copy link
Copy Markdown
Contributor

Introduces a new step in the build process which checks for circular dependencies to avoid issues in Node, as we saw in #931.
In fact, the check is done after removing all import * as ... since they are safe there.

It relies on madge in which support of ES6 modules has been added: pahen/madge#53.

The travis build is expected to fail here :)

@marclaval marclaval force-pushed the madge branch 3 times, most recently from 7c3e107 to 021ae67 Compare March 17, 2015 00:59
@marclaval
Copy link
Copy Markdown
Contributor Author

And it failed as expected, printing out the circular dependencies:

Starting 'build/checkCircularDependencies'...
[ [ 'angular2/src/core/compiler/element_binder',
    'angular2/src/core/compiler/view' ],
  [ 'angular2/src/core/compiler/pipeline/compile_control',
    'angular2/src/core/compiler/pipeline/compile_element',
    'angular2/src/core/compiler/element_binder',
    'angular2/src/core/compiler/view',
    'angular2/src/core/compiler/shadow_dom_strategy' ],
  [ 'angular2/src/core/compiler/pipeline/compile_element',
    'angular2/src/core/compiler/element_binder',
    'angular2/src/core/compiler/view',
    'angular2/src/core/compiler/shadow_dom_strategy' ] ]

@marclaval marclaval changed the title feat(build): check circular depencies in Node - WIP !!! feat(build): check circular depencies in Node.js Mar 17, 2015
@vsavkin vsavkin added @lgtm action: merge The PR is ready for merge by the caretaker labels Mar 18, 2015
@vsavkin vsavkin self-assigned this Mar 18, 2015
@vsavkin vsavkin removed action: merge The PR is ready for merge by the caretaker @lgtm labels Mar 18, 2015
@vsavkin
Copy link
Copy Markdown
Contributor

vsavkin commented Mar 18, 2015

@Mlaval This seems to be working, but I am not sure how it should be used.

When I run gulp build/checkCircularDependencies I see a list of cyclic dependencies. But when I run build.js.dev, I see no errors. Can you explain what this is for and how it works?

@mhevery mhevery added @lgtm action: merge The PR is ready for merge by the caretaker labels Mar 18, 2015
@marclaval marclaval closed this in a46af9c Mar 18, 2015
@marclaval
Copy link
Copy Markdown
Contributor Author

@vsavkin the build/checkCircularDependencies task only scans the /dist/js/dev/es6/ folder to find out circular dependencies, using Madge and ignoring the import * as .... It it finds some, it logs them and exits immediately.
It does not rebuild the /dist/js/dev/es6/ folder.

So my guess is that you ran the task with some old code in this folder, hence you got the list. Then, when you ran build.js.dev, it generated a fresh /dist/js/dev/es6/ where there was no more issues.

@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 13, 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 cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants