fix: escape backticks and dollar signs before creating interpolated string#15320
fix: escape backticks and dollar signs before creating interpolated string#15320
Conversation
🦋 Changeset detectedLatest commit: d2a411a 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 |
| * @param {string} str | ||
| * @returns {string} | ||
| */ | ||
| export function create_dynamic_string(name, args, str) { |
There was a problem hiding this comment.
maybe I should look at how this is used, but I don't quite understand why this is called create_dynamic_string. Would something like create_function_as_string be more accurate? or maybe the jsdoc could be filled out a bti more to give a better understanding
There was a problem hiding this comment.
Yeah I'd like better JSDoc on this
There was a problem hiding this comment.
I've renamed it and documented it. How's this? d2a411a
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
| import { assert, test } from 'vitest'; | ||
| import { create_dynamic_string } from './utils.js'; | ||
|
|
||
| test('create_dynamic_string escapes backslashes', () => { |
There was a problem hiding this comment.
Should test for dollar signs too.
There was a problem hiding this comment.
The escaping is done in a separate function and is tested in https://github.com/sveltejs/kit/pull/15320/changes#diff-ee61d627d800b208617d7a722797dc2636f588c2909ea779b52d3db38fd75d20R21
This makes the code kind of roundabout since the escaping and function creation isn't in the same place. But we needed to escape the CSS before we add the placeholders ${example}. Otherwise, the needed dollar signs would have been escaped too. https://github.com/sveltejs/kit/pull/15320/changes#diff-6198c333fe881f783129119271d28c82da0710a57556a80a844eaf27ee0681b3R63
We could insert our own placeholders instead such as __SVELTEKIT_ASSET__, then do the escaping and placeholder replacements in the util function. But that also seems a bit roundabout. Is there a better way to go about this?
| * @param {string} str | ||
| * @returns {string} | ||
| */ | ||
| export function create_dynamic_string(name, args, str) { |
There was a problem hiding this comment.
Yeah I'd like better JSDoc on this
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @sveltejs/adapter-cloudflare@7.2.8 ### Patch Changes - fix: skip comment lines in `_redirects` file ([#15325](#15325)) - Updated dependencies \[[`e87efba`](e87efba), [`71ddbc7`](71ddbc7), [`1bae374`](1bae374), [`20dfadf`](20dfadf), [`8c2384a`](8c2384a)]: - @sveltejs/kit@2.52.1 ## @sveltejs/adapter-netlify@6.0.1 ### Patch Changes - fix: export Netlify config directly from the instrumented serverless function ([#15335](#15335)) - Updated dependencies \[[`e87efba`](e87efba), [`71ddbc7`](71ddbc7), [`1bae374`](1bae374), [`20dfadf`](20dfadf), [`8c2384a`](8c2384a)]: - @sveltejs/kit@2.52.1 ## @sveltejs/kit@2.52.1 ### Patch Changes - fix: clear stale preflight issues on subsequent valid form submissions ([#15281](#15281)) - chore: remove dependency on `sade` ([#15272](#15272)) - fix: include `.txt` files in precompression ([#15259](#15259)) - fix: escape backticks and dollar signs when creating inlined css ([#15320](#15320)) - fix: increment `form.pending` count before preflight validation ([#15279](#15279)) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…tring (sveltejs#15320) closes sveltejs#15319 This PR fixes an edge case where the user's CSS needs to be inlined but has backticks and possible string interpolation syntax. We solve this by escaping the backtick and dollar sign so that those don't interfere with our actual string interpolation when we create the dynamic CSS module. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @sveltejs/adapter-cloudflare@7.2.8 ### Patch Changes - fix: skip comment lines in `_redirects` file ([sveltejs#15325](sveltejs#15325)) - Updated dependencies \[[`e87efba`](sveltejs@e87efba), [`71ddbc7`](sveltejs@71ddbc7), [`1bae374`](sveltejs@1bae374), [`20dfadf`](sveltejs@20dfadf), [`8c2384a`](sveltejs@8c2384a)]: - @sveltejs/kit@2.52.1 ## @sveltejs/adapter-netlify@6.0.1 ### Patch Changes - fix: export Netlify config directly from the instrumented serverless function ([sveltejs#15335](sveltejs#15335)) - Updated dependencies \[[`e87efba`](sveltejs@e87efba), [`71ddbc7`](sveltejs@71ddbc7), [`1bae374`](sveltejs@1bae374), [`20dfadf`](sveltejs@20dfadf), [`8c2384a`](sveltejs@8c2384a)]: - @sveltejs/kit@2.52.1 ## @sveltejs/kit@2.52.1 ### Patch Changes - fix: clear stale preflight issues on subsequent valid form submissions ([sveltejs#15281](sveltejs#15281)) - chore: remove dependency on `sade` ([sveltejs#15272](sveltejs#15272)) - fix: include `.txt` files in precompression ([sveltejs#15259](sveltejs#15259)) - fix: escape backticks and dollar signs when creating inlined css ([sveltejs#15320](sveltejs#15320)) - fix: increment `form.pending` count before preflight validation ([sveltejs#15279](sveltejs#15279)) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
closes #15319
This PR fixes an edge case where the user's CSS needs to be inlined but has backticks and possible string interpolation syntax. We solve this by escaping the backtick and dollar sign so that those don't interfere with our actual string interpolation when we create the dynamic CSS module.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits