-
Notifications
You must be signed in to change notification settings - Fork 97
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
Corrected "TypeError: Cannot assign to read only property", closes #68 #75
Conversation
@danielgindi Thanks for creating a PR. How did you test it? I saw that there is a merged PR in a forked repository: https://github.com/alist-org/jschardet/pull/1. Has this been used for testing? |
I've tested it in a production system with a few inputs that failed due to this error, and got detected correctly with this PR. |
@danielgindi Thanks for your response. How did you include your changed version of jschardet in your project? I installed jschardet using npm in my project, but AFAIK that does not work without a release of this PR. @aadsm Does an example project (with the fixed version of jschardet) help you to review the PR? Are manual changes to the codebase even the way to go? Or is this codebase still/regularly generated/ported based on the Python codebase? |
Just put the git url of the PR branch as the package version. |
Let me see if I can check this tomorrow, otherwise on Wednesday where I'll have more time. If I don't please ping me again. |
@aadsm Friendly reminder. Don't hurry. Take your time. We are all busy 😉 Thanks for creating and maintaining this project |
If you could add a test here that would be great. Ignore the qunit stuff, that's what I used at the time, but I've just ported them to jest. I only (manually) converted this from python when I created the project 12 years ago. |
I've just checked the source line with the issue. I guess this is what I get for not using type checkers, although at the time none existed for javascript. |
Hello @aadsm Do you plan to publish this change in a new version soon? |
Changes since 3.1.6: 2698f33 Version 3.1.6 (patch)\nd500c29 Version 3.1.5 (patch)\n1d1ff49 Version 3.1.4 (patch)\n3a2d31b Version 3.1.3 (patch)\n08d9559 Version 3.1.2 (patch)\n2f9d07c Version 3.1.1 (patch)\nef7d0da Version 3.1.0 (minor) \\n \\n Changes since git tag --list v* --sort=-taggerdate | head -1: \\n\n1e8ceb3 3.0.1\n4d45864 Updated browserify and jest versions\n56e10ee Cosmetic changes to the way the percentage is shown in show-size-changes.sh\n7010531 Add script to determine size changes in the distributed files\n120c0e7 npm audit --fix\na1c0607 Merge pull request #75 from danielgindi/fix/string_as_array\n2b9b07d Corrected \"TypeError: Cannot assign to read only property\", closes #68\n5a57c11 Convert the encodings tests to jest\n08e6c81 Add initial set of jest tests\n7897929 Check if given encodings exist against the denormalized list of supported encodings\naf66fa6 Add typescript support to detectEncodings\n9b49243 Add detectEncondings option\ne5945e2 Merge branch master of https://github.com/aadsm/jschardet\n71bcf43 npm audit fix\n094cb6f Merge pull request #71 from bpasero/patch-1\nc089b44 Add `detectAll` to index.d.ts\n0ae9a3c Update package-lock.json\nf71723b Add .npmignore to exclude tests
Changes since 3.1.6: 296ee53 3.2.0 2698f33 Version 3.1.6 (patch) d500c29 Version 3.1.5 (patch) 1d1ff49 Version 3.1.4 (patch) 3a2d31b Version 3.1.3 (patch) 08d9559 Version 3.1.2 (patch) 2f9d07c Version 3.1.1 (patch) ef7d0da Version 3.1.0 (minor) \n \n Changes since git tag --list 'v*' --sort=-taggerdate | head -1: \n 1e8ceb3 3.0.1 4d45864 Updated browserify and jest versions 56e10ee Cosmetic changes to the way the percentage is shown in show-size-changes.sh 7010531 Add script to determine size changes in the distributed files 120c0e7 npm audit --fix a1c0607 Merge pull request #75 from danielgindi/fix/string_as_array 2b9b07d Corrected "TypeError: Cannot assign to read only property", closes #68 5a57c11 Convert the encodings tests to jest 08e6c81 Add initial set of jest tests 7897929 Check if given encodings exist against the denormalized list of supported encodings af66fa6 Add typescript support to detectEncodings 9b49243 Add detectEncondings option e5945e2 Merge branch 'master' of https://github.com/aadsm/jschardet 71bcf43 npm audit fix 094cb6f Merge pull request #71 from bpasero/patch-1 c089b44 Add `detectAll` to index.d.ts 0ae9a3c Update package-lock.json f71723b Add .npmignore to exclude tests
Changes since 3.1.6: 2698f33 Version 3.1.6 (patch) d500c29 Version 3.1.5 (patch) 1d1ff49 Version 3.1.4 (patch) 3a2d31b Version 3.1.3 (patch) 08d9559 Version 3.1.2 (patch) 2f9d07c Version 3.1.1 (patch) ef7d0da Version 3.1.0 (minor) \n \n Changes since git tag --list 'v*' --sort=-taggerdate | head -1: \n 1e8ceb3 3.0.1 4d45864 Updated browserify and jest versions 56e10ee Cosmetic changes to the way the percentage is shown in show-size-changes.sh 7010531 Add script to determine size changes in the distributed files 120c0e7 npm audit --fix a1c0607 Merge pull request #75 from danielgindi/fix/string_as_array 2b9b07d Corrected "TypeError: Cannot assign to read only property", closes #68 5a57c11 Convert the encodings tests to jest 08e6c81 Add initial set of jest tests 7897929 Check if given encodings exist against the denormalized list of supported encodings af66fa6 Add typescript support to detectEncodings 9b49243 Add detectEncondings option e5945e2 Merge branch 'master' of https://github.com/aadsm/jschardet 71bcf43 npm audit fix 094cb6f Merge pull request #71 from bpasero/patch-1 c089b44 Add `detectAll` to index.d.ts 0ae9a3c Update package-lock.json f71723b Add .npmignore to exclude tests
Changes since 3.0.0: f3de69b Updated bundles 25f107a Updated github workflow files 4d45864 Updated browserify and jest versions 56e10ee Cosmetic changes to the way the percentage is shown in show-size-changes.sh 7010531 Add script to determine size changes in the distributed files 120c0e7 npm audit --fix a1c0607 Merge pull request #75 from danielgindi/fix/string_as_array 2b9b07d Corrected "TypeError: Cannot assign to read only property", closes #68 5a57c11 Convert the encodings tests to jest 08e6c81 Add initial set of jest tests 7897929 Check if given encodings exist against the denormalized list of supported encodings af66fa6 Add typescript support to detectEncodings 9b49243 Add detectEncondings option e5945e2 Merge branch 'master' of https://github.com/aadsm/jschardet 71bcf43 npm audit fix 094cb6f Merge pull request #71 from bpasero/patch-1 c089b44 Add `detectAll` to index.d.ts 0ae9a3c Update package-lock.json f71723b Add .npmignore to exclude tests Bundle size changes since v3.0.0: * dist/jschardet.js +3135 (465888 -> 469023) * dist/jschardet.min.js +3460 +0.01% (335803 -> 339263)
No description provided.