fix(compiler): log warning for missing name/namespace #4825
Conversation
nolanlawson
changed the title
|
This breaks |
nolanlawson
changed the title
| ' or filename ' + | ||
| JSON.stringify(filename) | ||
| ); | ||
| } |
There was a problem hiding this comment.
I can't figure out how to test this since we've only seen Vite do this.
There was a problem hiding this comment.
I'm using this same logic here:
And the unit test has been reported to fail on windows:
That's actually not a rollup or vite plugin, just an experiment with custom elements manifest, so I'm thinking path.dirname(filename).split(path.sep).slice(-2); is a problem on Windows.
There was a problem hiding this comment.
path.sep is different on both systems so it's quite possible we should just do / instead. I'm looking at parseDescriptorFromFilePath and it seems we only ever pass Rollup identifiers into it, and I'm wondering if Rollup just normalizes these to use / rather than \ on Windows.
There was a problem hiding this comment.
The Rollup docs don't specify but given that this is an id I'm assuming it's the identifier of the imported modules, e.g. import 'x/foo' would be "x/foo". So it's always going to be a forward slash. This is probably the issue.
* test(ssr): add tests for nested elements in slots (#5048) * fix(compiler): log warning for missing name/namespace (#4825) * test(karma): remove unnecessary IE11-related code (#5054) * fix: replace barrel exports from `lwc` with `@lwc/ssr-runtime` (#5034) * fix: replace barrel from `lwc` package with '@lwc/ssr-runtime' * fix: handle * barrel case and corresponding tests * fix: function naming * fix: barrel import test parity * fix: include optional exported alias for export all declaration replacement, tests * chore: explain function name massaging in test * fix: deep clone objects and optimize tests * fix: remove unused shared file * test(karma): add test for for:each issue #4889 (#5053) * fix(ssr): missing bookends for slotted lwc:if not at the top-level (#5027) Co-authored-by: Nolan Lawson <[email protected]> * fix(ssr): fix HTML comment bookends for if blocks (#5055) Co-authored-by: Will Harney <[email protected]> * fix(ssr-compiler): namespace and name should be optional in ComponentTransformOptions (#5058) * test(ssr): test `if` with adjacent text (#5056) * test(karma): reduce #4889 even further (#5060) * fix(ssr): fix `style` attribute rendering (#5061) * fix(ssr-compiler): harmonize some wire errors (#5062) Co-authored-by: Will Harney <[email protected]> * fix: only call callback when needed @W-17420330 (#5064) * fix: only call callback when needed @W-17420330 * chore: simplify test * fix: use correct class check * fix(ssr): render from superclass (#5063) Co-authored-by: Nolan Lawson <[email protected]> * test(ssr): add more superclass tests (#5065) * fix: use correct shadow root @W-17441501 (#5070) * fix: use correct shadow root @W-17441501 * chore: yagni i guess * chore: 🛩️📦 * If you read this, tell me so! * fix(ssr): align csr and ssr reflective behavior (#5050) * chore: release v8.12.2 @W-17485572 (#5075) --------- Co-authored-by: Nolan Lawson <[email protected]> Co-authored-by: jhefferman-sfdc <[email protected]> Co-authored-by: Matheus Cardoso <[email protected]> Co-authored-by: Nolan Lawson <[email protected]> Co-authored-by: Eugene Kashida <[email protected]>
Details
Partially addresses #4824
Given that throwing an error here broke a downstream Nucleus test, I figure we should just make this a warning for now.
Does this pull request introduce a breaking change?
No breakages
If you are not passing in the
name/namespaceto@lwc/compilerproperly then it will warn.Does this pull request introduce an observable change?
No observables changes