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

Unable to override custom symbols translations #176

Closed
gurpreetbaidwan opened this issue Sep 25, 2023 · 12 comments
Closed

Unable to override custom symbols translations #176

gurpreetbaidwan opened this issue Sep 25, 2023 · 12 comments

Comments

@gurpreetbaidwan
Copy link
Contributor

gurpreetbaidwan commented Sep 25, 2023

Unable to override symbols using custom translation text. I couldn't find any examples to do so based on the new definition of symbols object. This is the best I could think of.

const symbols = {
         iec: { bits: [], bytes: [] },
         jedec: {
             bits: [],
             bytes: [
                  "bytes",
                  "KB",
                  "MB",
                  "GB",
             ]
         }
     };
const size = partial({ symbols: symbols, base: 2, standard: "jedec" });
return size(value || 0);
@gurpreetbaidwan
Copy link
Contributor Author

gurpreetbaidwan commented Sep 25, 2023

Created a PR with proposed solution #175

@avoidwork
Copy link
Owner

Hi, I merged your PR assuming you ran the tests, but most tests failed, so it was force-pushed out. I'm adding husky to enforce pre-commit actions.

> filesize@10.1.0 mocha
> mocha test/*.js



  Testing functionality
    1) It should pass base2 tests
    2) It should pass base2 JEDEC tests
    3) It should pass base10 tests
    4) It should pass base10 IEC tests
    √ It should pass invalid input tests
    5) It should pass symbols tests
    6) It should pass partial() tests
    7) It should pass bits tests
    8) It should pass full forms tests
    9) It should pass exponent tests
    10) It should pass separator tests
    11) It should pass locale tests
    12) It should pass localeOptions tests
    13) It should pass roundingMethod tests
    14) It should pass precision tests
    15) It should pass defaults tests
    16) It should pass BigInt tests


  1 passing (11ms)
  16 failing

  1) Testing functionality
       It should pass base2 tests:
     TypeError: Cannot read properties of undefined (reading 'bytes')
      at filesize (file:///C:/Users/jason/Projects/filesize.js/dist/filesize.esm.js:158:31)
      at Context.<anonymous> (file:///C:/Users/jason/Projects/filesize.js/test/filesize_test.js:21:16)
      at process.processImmediate (node:internal/timers:478:21)

@avoidwork
Copy link
Owner

avoidwork commented Oct 1, 2023

This is actually a really old feature (3.0.0)*; I removed the block of samples from the README because it was an outdated gist.

The tests are the easiest place to see how it's used:

it("It should pass symbols tests", function () {
	assert.equal(filesize(this.byte, {base: 10, symbols: {B: "Б"}}), "1 Б", "Should be '1 Б'");
	assert.equal(filesize(this.kibibyte, {base: 10, symbols: {B: "Б"}}), "1.02 kB", "Should be '1.02 kB'");
});

@avoidwork
Copy link
Owner

avoidwork commented Oct 1, 2023

I've returned the gist content to the README; that file & the dist outputs need to be updated.

You only need to pass symbols or fullforms depending on intention, unless you have a reactive UI where you can toggle between the two.

filesize(1, {symbols: {B: "Б"}});     // "1 Б"
filesize(12, {fullform: true, fullforms: ["байтов"]});  // "12 байтов"

@avoidwork
Copy link
Owner

Closing this issue, with the intention to revisit the README.md.

@gurpreetbaidwan
Copy link
Contributor Author

gurpreetbaidwan commented Oct 2, 2023

Hi @avoidwork
You suggested code is exactly how I was passing symbols before but it stopped to work after updating to latest version of filesize npm, as the symbols definition has changed.

Typescript code below

image

image

@avoidwork avoidwork reopened this Oct 2, 2023
@gurpreetbaidwan
Copy link
Contributor Author

Perhaps only the symbols option Type needs to be fixed and code would work as expected as per your example.

@gurpreetbaidwan
Copy link
Contributor Author

gurpreetbaidwan commented Oct 2, 2023

Created another PR #177 with just type definition fix.
All tests pass :)

@avoidwork
Copy link
Owner

avoidwork commented Oct 2, 2023

@tschlechtweg, as the PR (#174) author who introduced the definition, is this looser version going to cause an issue?

@ghost
Copy link

ghost commented Oct 2, 2023

@avoidwork I would recommend if the object takes abritary strings as keys and values to atleast restrict it to symbols? : { [ s: string ]: string | string[] ] }, but seeing that I may miss another use-case, that is then not covered, for now a looser definition would be fine.

@ghost
Copy link

ghost commented Oct 2, 2023

@avoidwork It will not cause an issue with the return type infering, such that I do not have arguments against it.

@gurpreetbaidwan
Copy link
Contributor Author

Thank you @avoidwork & @tschlechtweg
:)

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

No branches or pull requests

2 participants