Fix style compilation for file paths containing webpack template strings #1247
Conversation
🦋 Changeset detectedLatest commit: 0e17aac The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -2,7 +2,7 @@ | |||
| "compilerOptions": { | |||
| "target": "ESNEXT" /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017','ES2018' or 'ESNEXT'. */, | |||
| "module": "commonjs" /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', or 'ESNext'. */, | |||
| "lib": ["es2019", "es2017", "dom"], | |||
| "lib": ["es2021", "dom"], | |||
There was a problem hiding this comment.
Required for tsc to recognize replaceAll. It was added in node 15, so IMO it's safe to use.
|
This would be a very welcome fix! |
…ct into webpack-square-brackets-fix
…ct into webpack-square-brackets-fix
|
Not quite sure where to put tests. Just in a random fixture, or should we make a webpack/nextjs specific one? #1203 sort of already paved the way for this a bit, although it's next-specific. |
|
I've added unit tests and a fixture for the escaping functionality. |
| const templateStringRegexp = /\[([^\[\]\.]+)\]/g; | ||
|
|
||
| export const escapeWebpackTemplateString = (s: string) => | ||
| s.replaceAll(templateStringRegexp, '[\\$1\\]'); |
There was a problem hiding this comment.
I went with the $1 syntax as my assumption is that it's less overhead compared to creating a replacement function.
| } from '@vanilla-extract-private/test-helpers'; | ||
|
|
||
| import test from './fixture'; | ||
| import { webpack as testCases } from './testCases'; |
There was a problem hiding this comment.
Chose to restrict this fixture to only run using webpack.
Fixes #1069.
You'd think this would be easily google-able, but apparently not. Eventually found out how to escape these template strings in the webpack docs, but only after digging through the source code of
TemplatedPathPluginand working backwards from there.This feels like a rather naive fix to me. I'm not sure if changing this output filename could have any unintended side-effects. Also, I wanted to add a test case to one of the fixtures but wasn't sure which one. Wouldlow-levelbe the best place for it?Using a more robust regex now, and added some unit tests and a webpack-specific fixture.