Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

[WIP, please help improve] Fix #699 when importing css using postcss #1309

Closed
wants to merge 2 commits into from
Closed

[WIP, please help improve] Fix #699 when importing css using postcss #1309

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8fc1f90
Fix #699 when importing css using postcss
Evertt Jul 9, 2020
60a299b
#699, slightly more bug-proof
Evertt Jul 9, 2020
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
10 changes: 7 additions & 3 deletions src/core/create_compilers/RollupCompiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,14 @@ export default class RollupCompiler {
this.chunks.push(chunk);
},
transform: (code: string, id: string) => {
if (/\.css$/.test(id)) {
this.css_files.push({ id, code });
return ``;
if (!/\.css$/.test(id)) return;

if (/styleInject/.test(code)) {
Copy link
Member

@benmccann benmccann Jul 9, 2020

Choose a reason for hiding this comment

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

There's a lot to comprehend regarding this bug and I'm not sure I totally understand it yet because I'm not that familiar with PostCSS, etc. Looking for styleInject seems too specific to this usecase though and I think it'd be better handled in a more generic way. The first line of this function is checking to make sure that the code is CSS. Perhaps the issue here is that you actually have JavaScript in a file with a .css extension? It seems weird to me that you would have that, but perhaps the fix should be to check if the file contents are JavaScript instead of CSS? But I'm not quite sure a good way of doing that. Looking for something like export seems a bit better, but also seems like it could cause problems. E.g. what if the user has some data export functionality on their site and has a CSS class named .export?

return code;
Copy link
Member

@benmccann benmccann Jul 9, 2020

Choose a reason for hiding this comment

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

What's the difference between return; like on the first line and return code; that you have here?

Copy link
Author

@Evertt Evertt Jul 9, 2020

Choose a reason for hiding this comment

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

In this case I'm just mimicking the code that was there previously. That also either returned nothing (if the id did not test positive for /\.css$/) or it returned an empty string.

}

this.css_files.push({ id, code });
return ``;
}
});

Expand Down