X Tutup
Skip to content

feat(compiler): add schema support for elements#3225

Closed
pkozlowski-opensource wants to merge 1 commit intoangular:masterfrom
pkozlowski-opensource:schema_registry
Closed

feat(compiler): add schema support for elements#3225
pkozlowski-opensource wants to merge 1 commit intoangular:masterfrom
pkozlowski-opensource:schema_registry

Conversation

@pkozlowski-opensource
Copy link
Copy Markdown
Member

@pkozlowski-opensource pkozlowski-opensource commented Jul 23, 2015

This PR adds all the infrastructure for schema support (#2014), without plugging it into the existing compiler (as we need to have hand-crafted schema for Dart).

Here the basic info about the design / reasoning:

  • schema is "global" since all the default HTML elements and web components are "global", so repeating the same info in each and every @view would be cumbersome
  • There are 2 main interfaces:
    • ElementSchemaRegistry - "knows" schema for all the elements in the entire system and handles "extends"
    • ElementSchemaEntry - "knows" schema for one element type
export class ElementSchemaRegistry {
  hasProperty(elName: string, propName: string): boolean { return true; }

  getMappedPropName(elName: string, propName: string): string { return propName; }
}
export interface ElementSchemaEntry {
  getTagName(): string;
  extendsElement(): string;
  hasProperty(propName: string): boolean;
  getMappedPropName(propName: string): string;
}

Currently there are 2 implementations of the ElementSchemaEntry: "static" (hand-written) and "reflective" (to be used on the JS side). One registry can combine entries of any types.

@mhevery @tbosch could you please review? I would also like to discuss how we could plug this into the existing compiler (reflective impl will work in JS but will fail in Dart and we need to decide who / when should to hand-craft schema for Dart)

@pkozlowski-opensource pkozlowski-opensource added this to the alpha-32 milestone Jul 23, 2015
@pkozlowski-opensource pkozlowski-opensource added action: discuss action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 23, 2015
@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Jul 23, 2015
Closed
2 tasks
@naomiblack naomiblack modified the milestones: alpha-32, alpha-33 Jul 23, 2015
mhevery
mhevery reviewed Jul 24, 2015
View reviewed changes
modules/angular2/src/render/dom/schema/element_schema_entry.ts
Copy link
Copy Markdown
Contributor

@mhevery mhevery Jul 24, 2015

Choose a reason for hiding this comment

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

Why do we need an interface with methods? Why not just field names?

Copy link
Copy Markdown
Member Author

@pkozlowski-opensource pkozlowski-opensource Jul 27, 2015

Choose a reason for hiding this comment

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

Changed to fields. One downside of this is that instances of the ElementSchemaEntry are mutable now.

@mhevery
Copy link
Copy Markdown
Contributor

mhevery commented Jul 24, 2015

I assume there is a fallow up SHA since this is not hooked up to anything?

What about renames? I think we need some API which will allow the schema to communicate that inner-html maps to innerHTML

@mhevery mhevery 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: discuss labels Jul 24, 2015
@pkozlowski-opensource pkozlowski-opensource force-pushed the schema_registry branch 2 times, most recently from 96fd66c to fc16e58 Compare July 24, 2015 15:15
@pkozlowski-opensource
Copy link
Copy Markdown
Member Author

pkozlowski-opensource commented Jul 24, 2015

@mhevery I did all the renames, your proposals sound better to me as compared to the initial naming.

I think we need some API which will allow the schema to communicate that inner-html maps to innerHTML

Yes, this is what getMappedPropName(propName: string) is for: fc16e58#diff-0f6b3a73fc554443f84a8b76b568e918R21

I assume there is a fallow up SHA since this is not hooked up to anything?

Definitively. But I would like to discuss how to plug things in the Dart version, as I assume that we will need hand-written schema for Dart?

@pkozlowski-opensource pkozlowski-opensource removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jul 27, 2015
@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Jul 27, 2015

Pawel, I think we should also add information about events.
Same is true for attributes.

Note sure if we need it, but knowing the type of properties would also be nice... (e.g. boolean / string / object)

@pkozlowski-opensource
Copy link
Copy Markdown
Member Author

pkozlowski-opensource commented Jul 28, 2015

Pawel, I think we should also add information about events.
Same is true for attributes.

Definitively! I just want to put in place the basic infrastructure first that we can gradually extend. The main blocker for me for now is taking a decision about Dart - since we need to have a hand-written schema we need to coordinate here.

@pkozlowski-opensource pkozlowski-opensource mentioned this pull request Jul 28, 2015
Closed
@pkozlowski-opensource
Copy link
Copy Markdown
Member Author

pkozlowski-opensource commented Jul 29, 2015

Closing in favor of #3330

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 6, 2019

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

Reviewers

No reviews

Assignees

@pkozlowski-opensource pkozlowski-opensource

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer cla: yes

Projects

None yet

Milestone

alpha-33

Development

Successfully merging this pull request may close these issues.

5 participants

@pkozlowski-opensource @mhevery @tbosch @naomiblack @googlebot
X Tutup