Upgrade: Rewrite imports of relative files to use relative file paths

[philipp-spiess]

When we implemented the CSS import resolution system, we found out a detail about CSS imports in that files without a relative path prefix would still be relative to the source file. E.g.:

@import 'foo.css';

Should first look for the file foo.css in the same directory. To make this cost as cheap as possible, we limited this by a heuristics to only apply the auto-relative imports for files with a file extension.

Naturally, while testing v4 on more templates, we found that it's common for people to omit the file extension when loading css file. The above could also be written as such:

@import 'foo';

To improve this, we have two options:

• We either remove the heuristics, making every @import more expensive because we have to check for relative files.
• We upgrade our codemods to rewrite @import statements to be explicitly relative.

Because we really care about performance, we opted to go with the latter option. This PR adds the codemod and removes the heuristics so we resolve CSS files similar to how you would resolve JS files.

[graphite-app]

Your org has enabled the Graphite merge queue for merging into next

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[philipp-spiess]

• Upgrade: Rewrite imports of relative files to use relative file paths #14755 👈
• next

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @philipp-spiess and the rest of your teammates on Graphite

[RobinMalfait]

Looks good, we do bail when it starts with . which means that

@import "./foo"

Stays as-is instead of resulting in:

@import "./foo.css"

[philipp-spiess]

Merge activity

• Oct 22, 12:07 PM EDT: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Oct 22, 12:09 PM EDT: A user added this pull request to the Graphite merge queue.
• Oct 22, 12:30 PM EDT: The Graphite merge queue couldn't merge this PR because it had conflicts with the trunk branch.
• Oct 22, 12:46 PM EDT: A user added this pull request to the Graphite merge queue.
• Oct 22, 12:47 PM EDT: The Graphite merge queue couldn't merge this PR because it had conflicts with the trunk branch.

[adamwathan]

Can't quite tell from the changes — do we support this now or no?

@import "some-relative-file.css";

IMO we shouldn't if we do currently.

[RobinMalfait]

With this codemod, this will be converted to:

@import "./some-relative-file.css";

If the some-relative-file.css can be found on disk.

[adamwathan]

With this codemod, this will be converted to:

@import "./some-relative-file.css";

If the some-relative-file.css can be found on disk.

Yeah but say you don't run the codemod, does our @import handling handle this?

@import "some-relative-file.css";

IMO it should error.

[philipp-spiess]

@adamwathan Yeah this change removed the logic where it automatically appends the ./ so we now error with this in the code.

[lubomirblazekcz]

Both

With this codemod, this will be converted to:

@import "./some-relative-file.css";

If the some-relative-file.css can be found on disk.

Yeah but say you don't run the codemod, does our @import handling handle this?

@import "some-relative-file.css";

IMO it should error.

Both is valid css import and should work though. Just found out that this broke bunch of imports in my css sice the update to latest beta. Should I create a new issue?