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.

Sign up for GitHub

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

Jump to bottom

refactor(core): change module semantics #10164

Closed
wants to merge 1 commit into from
Closed

refactor(core): change module semantics #10164

wants to merge 1 commit into from

Conversation

tbosch
Copy link
Contributor

@tbosch tbosch commented Jul 19, 2016 •
edited
Loading

Main part of #10043

@tbosch tbosch changed the title wip: fix module semantics refactor(core): fix module semantics Jul 19, 2016
@tbosch tbosch added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 19, 2016
IgorMinar
IgorMinar reviewed Jul 19, 2016
View reviewed changes
modules/@angular/upgrade/src/upgrade_adapter.ts
const compilerFactory: CompilerFactory = platformRef.injector.get(CompilerFactory);
var moduleRef = bootstrapModuleFactory(
compilerFactory.createCompiler().compileAppModuleSync(DynamicModule), platformRef);
const boundCompiler: Compiler = moduleRef.injector.get(Compiler);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is it "bound" and what is it bound to? to the platform? but every compiler instance is bound in that way. no?

@lacolaco
Copy link
Contributor

Is there a design doc for new AppModules?

@tbosch tbosch force-pushed the fix_modules2 branch 3 times, most recently from 0f8ddf2 to 829c0b0 Compare July 23, 2016 13:21
@tbosch tbosch mentioned this pull request Jul 23, 2016
Closed
2 tasks
@tbosch tbosch changed the title refactor(core): fix module semantics refactor(core): change module semantics Jul 23, 2016
@tbosch tbosch force-pushed the fix_modules2 branch 5 times, most recently from 326db0f to 00a6e33 Compare July 23, 2016 20:12
@tbosch
Copy link
Contributor Author

tbosch commented Jul 23, 2016

This is ready to review now!

@tbosch
Copy link
Contributor Author

tbosch commented Jul 23, 2016

@IgorMinar @mhevery @vicb Please decide who should do the review...

@tbosch
Copy link
Contributor Author

tbosch commented Jul 24, 2016

/cc @juliemr For the testing relating changes, especially the (deprecated) support of setBaseTestProviders.

@tbosch tbosch mentioned this pull request Jul 25, 2016
Closed
IgorMinar
IgorMinar reviewed Jul 25, 2016
View reviewed changes
modules/@angular/compiler/src/offline_compiler.ts
ngModuleByComponent.set(dirMeta.type.runtime, ngModuleMeta);
}
});
ngModuleMeta.exportedDirectives.forEach((dirMeta) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

obsolete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

IgorMinar
IgorMinar reviewed Jul 25, 2016
View reviewed changes
tools/public_api_guard/core/index.d.ts
@@ -928,6 +868,68 @@ export declare class KeyValueDiffers {
export declare function lockRunMode(): void;

/** @experimental */
export declare var NgModule: NgModuleMetadataFactory;

/** @stable */
Copy link
Contributor

Choose a reason for hiding this comment

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

mark all as experimental for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@tbosch tbosch added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 25, 2016
@tbosch
Copy link
Contributor Author

tbosch commented Jul 25, 2016

Reviewed in person with @IgorMinar

@naomiblack
Copy link
Contributor

naomiblack commented Jul 25, 2016 •
edited
Loading

@laco0416 This document (http://g.co/ng/modules) explains the angular team's plan for how NgModules will work. Still in-progress -- we are validating this week.

@lacolaco
Copy link
Contributor

@naomiblack Thank you. I'll prepare for the new style.

@tbosch tbosch force-pushed the fix_modules2 branch 8 times, most recently from e56446a to 74841c9 Compare July 26, 2016 13:24
@tbosch
refactor(core): change module semantics
2cc839b 
This contains major changes to the compiler, bootstrap of the platforms
and test environment initialization.

Main part of angular#10043

BREAKING CHANGE:
- Semantics and name of `@AppModule` (now `@NgModule`) changed quite a bit.
  This is actually not breaking as `@AppModules` were not part of rc.4.
  We will have detailed docs on `@NgModule` separately.
- `coreLoadAndBootstrap` and `coreBootstrap` can't be used any more (without migration support).
  Use `bootstrapModule` / `bootstrapModuleFactory` instead.
- All Components listed in routes have to be part of the `declarations` of an NgModule.
  Either directly on the bootstrap module / lazy loaded module, or in an NgModule imported by them.
@tbosch tbosch closed this in 46b2127 Jul 26, 2016
@tbosch tbosch deleted the fix_modules2 branch July 26, 2016 14:46
@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 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@mhevery mhevery

@IgorMinar IgorMinar

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tbosch @lacolaco @naomiblack @IgorMinar @mhevery @vicb @googlebot