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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle empty xliff files #52

Merged
merged 2 commits into from
Feb 7, 2022
Merged

Handle empty xliff files #52

merged 2 commits into from
Feb 7, 2022

Conversation

dhoko
Copy link
Contributor

@dhoko dhoko commented Feb 7, 2022 •
edited

馃憢

I got an issue when reading an empty xliff file, the lib crashes

TypeError: Cannot read properties of undefined (reading '0')                                                      
    at xliff12ToJsClb (file://node_modules/xliff/esm/xliff12ToJs.js:32:35)
    at file:////node_modules/xliff/esm/xliff12ToJs.js:124:14                
    at new Promise (<anonymous>)                                                                                  
    at xliff12ToJs (file://node_modules/xliff/esm/xliff12ToJs.js:123:12)  ```

ex:

<?xml version="1.0" encoding="UTF-8"?>                                      
<xliff 
  xmlns="urn:oasis:names:tc:xliff:document:1.2" 
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
  version="1.2" 
  xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 http://docs.oasis-open.org/xliff/v1.2/os/xliff-core-1.2-strict.xsd"></xliff>

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test 158 passing (187ms)
  • tests are included
  • documentation is changed or added

dhoko and others added 2 commits February 7, 2022 15:16
@dhoko
Handle empty xliff files
e962f5c 
The lib should not break when we parse this xliff file:

```xliff
<?xml version="1.0" encoding="UTF-8"?>                                      
<xliff 
  xmlns="urn:oasis:names:tc:xliff:document:1.2" 
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
  version="1.2" 
  xsi:schemaLocation="urn:oasis:names:tc:xliff:document:1.2 http://docs.oasis-open.org/xliff/v1.2/os/xliff-core-1.2-strict.xsd"></xliff>                                                                          ```
Add test xliff 1.2 empty file
3a21a86
adrai
adrai reviewed Feb 7, 2022
View reviewed changes
lib/xliff12ToJs.js

return resources
}, {})
if (xliffRoot.elements && xliffRoot.elements.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you reverse this condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean?

if (!xliffRoot.elements || !xliffRoot.elements.length) {
  result.resources = {}
 } else ...

or

if (!xliffRoot.elements || !xliffRoot.elements.length) {
 result.resources = {}
 if (cb) return cb(null, result)
 return result
} 
...

Copy link
Contributor

@adrai adrai Feb 7, 2022 •
edited

Choose a reason for hiding this comment

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

ahh, sorry now I see.
Not easy from mobile phone 馃槈
All ok. Will merge asap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok np 馃榿

thx 馃檹

@adrai adrai merged commit c3a59e0 into locize:master Feb 7, 2022
@adrai
Copy link
Contributor

adrai commented Feb 7, 2022

Thank you for your contribution.
It's included in v6.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrai adrai adrai left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dhoko @adrai