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

add patch for text-encoding-utf-8, support utf8 #632

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Conversation

ahaoboy
Copy link
Contributor

@ahaoboy ahaoboy commented Jul 23, 2024

close: #630
If there is a better solution, please close this PR

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 23, 2024

Why use window instead of globalThis as the global object? Is it because events can be listened on window?

@saghul
Copy link
Owner

saghul commented Jul 23, 2024

We already use patch-package on the polyfill, can you please apply the patch there instead?

@saghul
Copy link
Owner

saghul commented Jul 23, 2024

Why use window instead of globalThis as the global object? Is it because events can be listened on window?

Out of habit, but yeah this should be globalThis.

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 23, 2024

We already use patch-package on the polyfill, can you please apply the patch there instead?

done

@ahaoboy ahaoboy marked this pull request as draft July 23, 2024 15:01
@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 23, 2024

The behavior of patch-package is a bit strange, I may need to investigate further, and it seems that after installing patch, the compilation result of make js && make does not change, which is a bit strange

@saghul
Copy link
Owner

saghul commented Jul 23, 2024

Yeah when the polyfills change the Makefile doesn't quite pick it up because the dependencies across files are not detected.

You can force it by deleting the .c file.

@ahaoboy
Copy link
Contributor Author

ahaoboy commented Jul 24, 2024

What's strange is that this patch doesn't seem to modify the source code, so why is it added here?
Is it because the editor automatically formats it?

@@ -639,4 +640,4 @@ function UTF8Encoder(options) {
 }
 
 exports.TextEncoder = TextEncoder;
-exports.TextDecoder = TextDecoder;
\ No newline at end of file
+exports.TextDecoder = TextDecoder;

I only added an alias utf8, which should cover most usage scenarios.

@ahaoboy ahaoboy marked this pull request as ready for review July 24, 2024 02:26
@saghul saghul merged commit d6f9888 into saghul:master Jul 24, 2024
18 checks passed
@saghul
Copy link
Owner

saghul commented Jul 24, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Encoding not supported. Only utf-8 is supported
2 participants