[types/fbt] Added missing typings for latest version of fbt@0.16.x #55702

retyui · 2021-09-11T14:20:43Z

Please fill in this template.

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

Select one of these and delete the others:

If adding a new definition:

The package does not already provide its own types, or cannot have its .d.ts files generated via --declaration
If this is for an npm package, match the name. If not, do not conflict with the name of an npm package.
Create it with dts-gen --dt, not by basing it on an existing project.
Represents shape of module/library correctly
tslint.json should contain { "extends": "dtslint/dt.json" }, and no additional rules.
tsconfig.json should have noImplicitAny, noImplicitThis, strictNullChecks, and strictFunctionTypes set to true.

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: <>
If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.

If removing a declaration:

If a package was never on Definitely Typed, you don't need to do anything. (If you wrote a package and provided types, you don't need to register it with us.)
Delete the package's directory.
Add it to notNeededPackages.json.

- added tests; - added support TS 3.9; - split d.ts to support XML namespace in TS 4.2.

typescript-bot · 2021-09-11T14:21:20Z

@retyui Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

fbt — on npm, on unpkg
- owner-approval: @alexandernanberg
- 1 added owner: ✎@retyui

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by type definition owners, DT maintainers or others

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 55702,
  "author": "retyui",
  "headCommitOid": "d57b6eecca185752cf4f4275bf09835f82c8a397",
  "lastPushDate": "2021-09-11T14:19:16.000Z",
  "lastActivityDate": "2021-09-13T15:31:02.000Z",
  "mergeOfferDate": "2021-09-13T14:17:59.000Z",
  "mergeRequestDate": "2021-09-13T15:31:02.000Z",
  "mergeRequestUser": "retyui",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "popularityLevel": "Well-liked by everyone",
  "pkgInfo": [
    {
      "name": "fbt",
      "kind": "edit",
      "files": [
        {
          "path": "types/fbt/fbt-tests.tsx",
          "kind": "test"
        },
        {
          "path": "types/fbt/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/fbt/package.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/fbt/ts4.1/fbt-tests.tsx",
          "kind": "test"
        },
        {
          "path": "types/fbt/ts4.1/index.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/fbt/ts4.1/tsconfig.json",
          "kind": "package-meta-ok"
        },
        {
          "path": "types/fbt/ts4.1/tslint.json",
          "kind": "package-meta-ok"
        }
      ],
      "owners": [
        "alexandernanberg"
      ],
      "addedOwners": [
        "retyui"
      ],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "alexandernanberg",
      "date": "2021-09-13T14:17:17.000Z",
      "isMaintainer": false
    }
  ],
  "mainBotCommentID": 917415669,
  "ciResult": "pass"
}

typescript-bot · 2021-09-11T14:21:22Z

🔔 @alexandernanberg — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

retyui · 2021-09-11T14:33:26Z

@alexandernanberg looking at git history you just copy my legacy gist and extend JSX namespace to support XML namespaces syntax which was added in TS@4.2+

So I decided:

To update typing to the latest version fbt@0.16.0
Add more safe tests

Split definition to support older TS versions:

import {FbtParam} from 'fbt';

// Typescript 3.9...4.2 => Only camelcase syntax
<FbtParam name="name">{person.getName()}</FbtParam>

// Typescript 4.2+      => XML namespaces + camelcase syntax
<fbt:param name="name">{person.getName()}</fbt:param>

alexandernanberg · 2021-09-13T09:14:41Z

@retyui Mostly looks good to me, although not sure if it's worth adding support for older TS versions? I would assume most people are using at least 4.2 now. Supporting both means more maintenance and complecity which would be nice to avoid.

But if you want to push for that, I think the file names are incorrect. They should be ts4.2/index.d.ts not ts4.1/index.d.ts right?

retyui · 2021-09-13T11:09:09Z

@alexandernanberg

I think the file names are incorrect. They should be ts4.2/index.d.ts not ts4.1/index.d.ts right?

Check a package.json file:

    "typesVersions": {
        "<=4.1": { "*": ["ts4.1/*"] }
    }

And when you run tests you will see how folders spread by versions

yarn test fbt

$ dtslint types fbt
testing from 3.9 to 4.1 in /home/i/open-source/AllDefinitelyTyped/types/fbt/ts4.1
testing from 4.2 to 4.5 in /home/i/open-source/AllDefinitelyTyped/types/fbt
Done in 12.41s.

Read more: https://github.com/DefinitelyTyped/DefinitelyTyped#i-want-to-use-features-from-very-new-typescript-versions

alexandernanberg

Ah you're right! Cool haven't seen that before.

LGTM

retyui · 2021-09-13T15:31:02Z

Ready to merge

Swahvay · 2022-02-07T05:30:13Z

types/fbt/index.d.ts

-        locale: string;
-    }
+export interface Fbt {
+    (text: string, description: string, options?: FbtOptions): Output['fbt'];


@retyui, you've specified the output here as string[], but it should be string, right? CC @yrichard.

Ah, sorry, I can't easily check right now because I'm traveling, but also, I dropped my involvement with FBT, due to the lack of support from Facebook when in conjunction with Typescript... Sorry!

Swahvay · 2022-02-07T05:35:57Z

types/fbt/index.d.ts


-export interface Fbt {
-    (source: string, desc: string, options?: FBT.Options): FBT.FbtResult;


Here's the previous type definition, which is

interface FbtResult { toString: () => string; }

@Swahvay because fbt returns array of string:

fbt(`My name ${fbt.param('name', person.getName())}`) // ['Мое имя', person.getName()]

How do you use fbt as a function in code then? Do you always have to write fbt(…).join(‘’)?

Did some digging and fbt() returns an FBTResult object, which has a toString() method defined here: https://github.com/facebook/fbt/blob/main/runtime/shared/FbtResultBase.js#L90, which does return just a string.

Sorry, I must have had some transforms mixed up. I was getting that type definition but after some reworking it's disappeared and everything is working correctly. Sorry for the confusion.

Actually I'm sorry again. It's back and I'm not sure if there error is on my end or not anymore. I do know that fbt() should not and doesn't return a string[] though.

@Swahvay so what you propose to do?

I'm not sure, but I ended up choosing another i18n that has more builtin integrations, so don't take my problems as necessarily the fault of this typing change. Feel free to ignore if you think it's right as is.

[types/fbt] Added missing typings for latest version of fbt@0.16.x

d57b6ee

- added tests; - added support TS 3.9; - split d.ts to support XML namespace in TS 4.2.

typescript-bot added the Edits Owners This PR adds or removes owners label Sep 11, 2021

typescript-bot added this to Waiting for Code Reviews in New Pull Request Status Board Sep 11, 2021

retyui mentioned this pull request Sep 11, 2021

[feat] Add typescript definitions for fbt module facebook/fbt#261

Closed

retyui mentioned this pull request Sep 11, 2021

[Docs] Add typescript info facebook/fbt#296

Closed

alexandernanberg approved these changes Sep 13, 2021

View reviewed changes

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner labels Sep 13, 2021

typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Sep 13, 2021

typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board Sep 13, 2021

typescript-bot merged commit 771a345 into DefinitelyTyped:master Sep 13, 2021

retyui deleted the update-fbt-types branch September 13, 2021 15:31

typescript-bot removed this from Recently Merged in New Pull Request Status Board Sep 13, 2021

Swahvay reviewed Feb 7, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[types/fbt] Added missing typings for latest version of fbt@0.16.x #55702

[types/fbt] Added missing typings for latest version of fbt@0.16.x #55702

retyui commented Sep 11, 2021

typescript-bot commented Sep 11, 2021 •

edited

typescript-bot commented Sep 11, 2021

retyui commented Sep 11, 2021 •

edited

alexandernanberg commented Sep 13, 2021

retyui commented Sep 13, 2021 •

edited

alexandernanberg left a comment

retyui commented Sep 13, 2021

Swahvay Feb 7, 2022

yrichard Feb 8, 2022

Swahvay Feb 7, 2022

retyui Feb 7, 2022

Swahvay Feb 7, 2022

Swahvay Feb 7, 2022 •

edited

Swahvay Feb 8, 2022

Swahvay Feb 8, 2022 •

edited

ghost Feb 8, 2022

Swahvay Feb 13, 2022


		export interface Fbt {
		(source: string, desc: string, options?: FBT.Options): FBT.FbtResult;

[types/fbt] Added missing typings for latest version of fbt@0.16.x #55702

[types/fbt] Added missing typings for latest version of fbt@0.16.x #55702

Conversation

retyui commented Sep 11, 2021

typescript-bot commented Sep 11, 2021 • edited

1 package in this PR

Code Reviews

Status

typescript-bot commented Sep 11, 2021

retyui commented Sep 11, 2021 • edited

alexandernanberg commented Sep 13, 2021

retyui commented Sep 13, 2021 • edited

alexandernanberg left a comment

Choose a reason for hiding this comment

retyui commented Sep 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swahvay Feb 7, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swahvay Feb 8, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

typescript-bot commented Sep 11, 2021 •

edited

retyui commented Sep 11, 2021 •

edited

retyui commented Sep 13, 2021 •

edited

Swahvay Feb 7, 2022 •

edited

Swahvay Feb 8, 2022 •

edited