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

tests(cli): add test for improper unicode encoding #8162

Merged
merged 3 commits into from
Nov 22, 2020

Conversation

kitsonk
Copy link
Contributor

@kitsonk kitsonk commented Oct 28, 2020

Fixes #8161

(Or actually will fix when swc-project/swc#1188 is fixed, this is a test case to help ensure it is fixed at the moment)

@bartlomieju
Copy link
Member

@kitsonk please rebase, #8197 should include the fix

@kitsonk kitsonk changed the title [WIP] fix(cli): incorrect unicode encoding in swc tests(cli): add test for improper unicode encoding Oct 30, 2020
@kitsonk kitsonk marked this pull request as ready for review October 30, 2020 19:59
@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 30, 2020

@bartlomieju @kdy1 the test case is still failing because the output is not as expected/valid. swc is still modifying the input string.

@kdy1
Copy link

kdy1 commented Oct 31, 2020

@kitsonk I'm considering an option in swc_ecma_codegen to preserve input or emit ascii only. How do you think about it?

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 31, 2020

For us, we would always choose to preserve input, so yes. I am not sure what the advantages of re-encoding the input would be.

@kdy1
Copy link

kdy1 commented Nov 17, 2020

This will be fixed by swc-project/swc#1221

@kdy1
Copy link

kdy1 commented Nov 22, 2020

Can you try this again with the new version?

@kitsonk
Copy link
Contributor Author

kitsonk commented Nov 22, 2020

@kdy1 it still "modifies" the strings, but the way it re-encodes them exhibits the same behaviour, so I have updated the expected output to match the behaviour of swc.

@kdy1
Copy link

kdy1 commented Nov 22, 2020

It should modify string anymore.

There's a test for the exact same input.

deno/Cargo.lock

Lines 2380 to 2382 in 686a17f

[[package]]
name = "swc_ecma_codegen"
version = "0.41.0"

It seems like deno is using old version of codegen. It's fixed by v0.41.1. Can you try cargo update -p swc_ecma_codegen?

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@kitsonk kitsonk merged commit fec7fdc into denoland:master Nov 22, 2020
jannes pushed a commit to jannes/deno that referenced this pull request Dec 1, 2020
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.

Deno 1.5.0 bundle regression: bad Unicode character encoding
3 participants