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.

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

Prevent HTML-escaping of module specifiers #1365

Merged
merged 3 commits into from
Jun 14, 2023
Merged

Prevent HTML-escaping of module specifiers #1365

merged 3 commits into from
Jun 14, 2023

Conversation

simonihmig
Copy link
Collaborator

Switching a pnpm-based repo to its newer lockfileVersion: '6.0' caused node_modules to have folders containing an = equal sign (for patched packages), which breaks the build as that equal sign gets wrongly HTML-escaped to =, causing module not found errors.

This PR basically replicates the same fix as for ember-auto-import.

I was able to confirm - by patching the local file in node_modules with the same changes as in this PR - that it fixes the build issue I had.

@@ -11,4 +11,7 @@ handlebars.registerHelper('json-stringify', function (input: any, indent?: numbe
handlebars.registerHelper('eq', function (a: any, b: any) {
return a === b;
});
export const compile = handlebars.compile;

export function compile(input: string) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Technically, this changes the signature of this function, as you could previously pass custom options. But this was nowhere the case AFAICT. And it seems better to me to not expose this, but own the config internally here.

I can change that of course in case of disagreement!

@simonihmig
Copy link
Collaborator Author

Failing test scenarios seem to be related to #1366

@ef4
Copy link
Contributor

ef4 commented Mar 7, 2023

Previously I had done this in each place it came up by using triple curlies. I like your solution better, but we should remove all the triple curlies too so it's not confusing.

@simonihmig
Copy link
Collaborator Author

but we should remove all the triple curlies too so it's not confusing.

@ef4 done!

Still getting a bunch of failed CI jobs due to some network issues when installing node, even after re-running them...

@ef4
Copy link
Contributor

ef4 commented Jun 14, 2023

Thanks for working on this, I think it's ready to go.

@ef4 ef4 merged commit a4d18ab into main Jun 14, 2023
@ef4 ef4 deleted the prevent-escape branch June 14, 2023 11:47
@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants