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

Fix a component namespace and name parsing issue #5117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix a component namespace and name parsing issue #5117

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba87df0
Fix a component namespace and name parsing issue
Mr-VincentW Jan 9, 2025
d62ce4c
Simplified the fix
Mr-VincentW Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/@lwc/rollup-plugin/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ export default function lwc(pluginOptions: RollupLwcOptions = {}): Plugin {
const [namespace, name] =
// Note we do not need to use path.sep here because this filename contains
// a '/' regardless of Windows vs Unix, since it comes from the Rollup `id`
wjhsf marked this conversation as resolved.
Show resolved Hide resolved
specifier?.split('/') ?? path.dirname(filename).split('/').slice(-2);
specifier?.split('/') ?? /(?:([^\\/]+?)[\\/])?([^\\/]+?)(?:[\\/]\2)?(?:\.scoped)?\.[^.]+$/.exec(filename)?.slice(1);
Copy link
Contributor

@cardoso cardoso Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think using a regex here is the right approach.

However, in my case (on Windows), I found them cometimes containing \ as well.

Are you able to debug what specifier and filename are here in this case you mentioned?

Ideally this line remains unchanged, and the filename is normalized via simple String.replace previous to it.

The assumption is that rollup already does this normalization, which is why providing a log of the variables as well a minimal repo would be needed to figure out the proper fix.

Copy link
Author

@Mr-VincentW Mr-VincentW Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I've updated my fix to only fix the delimiter issue.

BTW, rollup does normalize the ids but not the importer, and the LWC rollup plugin has a custom resolveId hook in the plugin to resolves ids based on both importee (id) and importer, which is where the issue derives.

Copy link
Author

Choose a reason for hiding this comment

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

Here's a screenshot of what the specifire and filename look like:
image

The specifier is always null for "directory modules" as it's parsed from a URL search param which is only appended to "alias modules" by the resolveId hook.


/* v8 ignore next */
if (!namespace || !name) {
Expand Down