Skip to content

build: add missing bits for material-examples package #4887

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

Merged
merged 1 commit into from
May 31, 2017

Conversation

jelbourn
Copy link
Member

@jelbourn jelbourn commented May 30, 2017

Adds:

  • Missing flatModuleId from the ngc options
  • Missing typing for module in order to set moduleId
  • Makes templateUrl and styleUrls for example components match material
    components.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 30, 2017
@@ -0,0 +1 @@
declare var module: {id: string};
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this module.id available in plunkers?

Copy link
Member

Choose a reason for hiding this comment

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

The moduleId should be removed by the inline resources script. Why do we need it at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was part of my debugging process. It ended up not being the problem, but I left it because the material2 components themselves use it. However, I didn't think of the plunkers. It looks like I failed to update how those work at all. Fixing that now...

Copy link
Member

Choose a reason for hiding this comment

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

I see. Basically they shouldn't be necessary at all. It's fine to leave them for now though.

Just want to check. Does something need the moduleId in our development? (e.g Google internally)

Copy link
Member Author

@jelbourn jelbourn May 30, 2017

Choose a reason for hiding this comment

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

Yes- Google uses it

Copy link
Member

Choose a reason for hiding this comment

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

Alright that makes sense then. For the examples you removed the module.id. You can remove the typings file therefore too

@jelbourn
Copy link
Member Author

Updated to remove moduleId for examples

@kara kara changed the title build: add mising bits for material-examples package build: add missing bits for material-examples package May 30, 2017
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. One minor comment about the unused typing file.

Adds:
* Missing `flatModuleId` from the ngc options
* Makes templateUrl and styleUrls for example components match material
components.
@jelbourn jelbourn added action: merge The PR is ready for merge by the caretaker pr: merge safe labels May 31, 2017
@mmalerba mmalerba merged commit f169152 into angular:master May 31, 2017
@jelbourn jelbourn deleted the docs-examples branch September 13, 2017 04:38
@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 7, 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 PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants