Fixes #854 - Make every .marko file a UI component #855
Conversation
| if (this._childTextNormalized) { | ||
| return; | ||
| } | ||
|
|
||
| this._childTextNormalized = true; | ||
|
|
||
| var trimStartEnd = this._trimStartEnd === true; | ||
| var trimStartEnd = forceTrimStartEnd === true || this._trimStartEnd === true; |
There was a problem hiding this comment.
Is this change necessary? It looks like it isn't being used (commented out in src/components/taglib/TransformHelper/handleRootNodes.js).
src/compiler/config.js
Outdated
|
|
||
| /** | ||
| * Controls whether or not a key should be assigned to all HTML | ||
| * and csutom tags at compile-time. The default is `true` |
| @@ -34,6 +34,13 @@ module.exports = function beginComponent(componentsContext, component, isSplitCo | |||
| return componentDef; | |||
| } | |||
|
|
|||
| if (isImplicitComponent === true) { | |||
There was a problem hiding this comment.
Thoughts on moving this conditional above the one that is currently above it? Micro-optimization, but there's probably no sense in doing the bitwise operations above if this is true.
There was a problem hiding this comment.
It's actually going to be more common for the if condition on line 32 to evaluate to true so it's better to keep the if (isImplicitComponent === true) lower. Whether or not a UI component is implicit only matters if it is a top-level UI component outside of UI component that will re-render in the browser.
| context.setFlag('hasComponentVar'); | ||
|
|
||
| var getCurrentComponentVar = context.importModule('marko_getCurrentComponent', | ||
| this.getMarkoComponentsRequirePath('marko/components/taglib/helpers/getCurrentComponent')); |
There was a problem hiding this comment.
Can marko/components/taglib/helpers/getCurrentComponent.js be deleted now? I don't think it's being used anymore.
There was a problem hiding this comment.
Yes, we can delete it. Good catch.
| @@ -92,7 +99,7 @@ module.exports = function handleComponentBind(options) { | |||
| if (rendererModule) { | |||
| if (rendererModule.inlineId) { | |||
| markoComponentVar = rendererModule.inlineId; | |||
| } else { | |||
| } else if (!isImplicitComponent) { | |||
There was a problem hiding this comment.
It's not possible for this to fall into an else case right? Otherwise markoComponentVar would be undefined. Maybe that's intended.
There was a problem hiding this comment.
It's okay if markoComponentVar is undefined. That is handled by markoComponentVar || builder.literal({}) later in the file.
| <html><head><title>Welcome Frank<script>(function(){var w=window;w.$components=(w.$components||[]).concat({"w":[["s2",0,{},{"f":1}]],"t":["/marko-test$1.0.0/autotests/async-render/components-await-title/components/hello/index.marko"]})||w.$components})()</script></title></head><body><!--M#s2--><div>Hello</div><!--M/s2--></body></html> |
There was a problem hiding this comment.
Is it valid for this script tag to be inside the title?
There was a problem hiding this comment.
I've not noticed that was happening until now, but that is not going to be valid. I'll investigate.
| @@ -1,79 +0,0 @@ | |||
| module.exports = require('marko/legacy-components').defineComponent({ | |||
There was a problem hiding this comment.
This is a breaking change... one which I think we need to make. If we're going to forgo semver, we should at least add deprecation warnings in the master branch and also in Marko 3 for any patterns covered by these deleted tests.
There was a problem hiding this comment.
For Marko v4, how long do the warnings need to be on the next patch version before we move this out of beta?
| compiledFile = compiledFile.replace(/marko\/dist\//g, 'marko/src/'); | ||
| expect(compiledFile).to.deep.equal(expectedFile); | ||
| helpers.compare(removeMarkoVersionComment(compiledFile), '.js'); |
There was a problem hiding this comment.
In any other case, I would say stripping the comment is fine, but isn't this test supposed to be asserting the existence of the comment?
There was a problem hiding this comment.
Perhaps. I will take a closer look.
Description
See: #854
Checklist:
Disclaimer: Contributions via GitHub pull requests are gladly accepted from their original author. Along with any pull requests, please state that the contribution is your original work and that you license the work to the project under the project's open source license. Whether or not you state this explicitly, by submitting any copyrighted material via pull request, email, or other means you agree to license the material under the project's open source license and warrant that you have the legal authority to do so.