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

Recognize JS namespace pattern in TS #37321

Open
5 tasks done
mpawelski opened this issue Mar 10, 2020 · 3 comments
Open
5 tasks done

Recognize JS namespace pattern in TS #37321

mpawelski opened this issue Mar 10, 2020 · 3 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@mpawelski
Copy link

I short, I propose two things:

  1. In TS files recognize JavaScript namespace patterns that are recognized in JS files with --allowJs. Like var my = my || {}; my.app = my.app || {}; (can't navigate javascript with manual namespaces #7632)
  2. Recognize methods that create global JavaScript namespaces from provided string. For example calling Ext.na("Company.data") should have the same effect as writing it "by hand" with var Company = Company || {}; Company.data = Company.data || {};

Why 1.

It's now possible to have project with --allowJs flag when you can define such code in JS file and TS file recognizes it:

JS file:

var app = app || {};
app.pages = app.pages || {};
app.pages.admin = app.pages.admin || {};

app.pages.admin.mailing = (function () {
    return {
        /**
         * @param {string} email 
         */
        sendMail: function (email) { }
    }
})()

TS file:

(function(){
    app.pages.admin.mailing.sendMail("[email protected]")
})();

See what I get when I hover over mailing property in my TS file.
image

The problem is that I want to incrementally migrate old JavaScript project to Typescript and convert each JS file to TS. It's not so easy or obvious how to migrate JS file that use this namespaces pattern without adding a lot of type definitions that can be inferred in JS file with allowJs compiler flag. Why not recognize this pattern in Typescript files?

Why 2. and example

This is the actual pattern that my current project uses in JS files

namespace('app.pages.admin');
app.pages.admin.mailing = (function () {
    return {
        /**
         * @param {string} email 
         */
        sendMail: function (email) { }
    }
})()

namespace function is global function that basically creates global namespace objects. It's logically equivalent to writing var app = app || {}; app.pages = app.pages || {}; app.pages.admin = app.pages.admin || {}; from previous example.

I want to be able to change JS file to TS file and be able to use this code in new TS files (but with static safety that TS provides).

I propose that we recognize special type alias definition for which compiler will recognize this pattern and act as if this global "javascript namespace" (or "expando" object) was created. For example this type alias will be added to standard lib:

type JavaScriptNamespace = string;

Then I could declare global namespace function like this

declare function namespace(n : JavaScriptNamespace): void;

and this TS code would be valid:

namespace('app.pages.admin');
app.pages.admin.mailing = (function () {
    return {
        sendMail: function (email: string): void { }
    }
})()

It's a bit similar in spirit to ThisType. I mean compiler have special handling to some type. But if old compiler sees this type then nothing happens (it's just a string type).

Use cases and current "workarounds"

It's all about making migration of current JS project to TS easier. It's hard to convince my team (and even myself) that we should use TS when you need to write code like this to get the same behavior you had in JS but with type safety:

//file: global.d.ts
declare var app: NamespaceApp
declare function namespace(namespace: string): void;

interface NamespaceApp {
    pages: NamespacePages;
}
interface NamespacePages {
    admin: NamespaceAdmin;
}
interface NamespaceAdmin {
}

//file: module1.js
interface NamespaceAdmin {
    module1: {
        foo: (arg: string) => void;
        bar: (arg: string[]) => void;
    }
}

namespace("app.pages.admin")
app.pages.admin.module1 = (function () {
    return {
        foo: function (arg: string) {
        },
        bar: function (arg: string[]) {
        }
    }
})()

//file: module2.js
interface NamespaceAdmin {
    module2: {
        foo2: (arg: string) => void;
        bar2: (arg: string[]) => void;
    }
}

namespace("app.pages.admin")
app.pages.admin.module2 = (function () {
    return {
        foo2: function (arg: string) {
        },
        bar2: function (arg: string[]) {
        }
    }
})()

you need to write type definitions for you methods twice and split you type definitions into interfaces to merge them 🤮

My current workaround is using Typescript namespaces like this (I have two approaches, don't like either of them):

//first approach
namespace app1.pages.admin.mailing {
    function privateFunc(){}
    export function sendMail(email: string): void{
        privateFunc();
    }
}

//second approach
namespace app2.pages.admin {
    export const mailing = (function(){
        function privateFunc(){}
        function sendMail(email: string){}
        return {
            sendMail
        }
    })()    
}

There are many problems with this use of Typescript's namespace:

  • the generated code is bigger, because this is how TS's namespaces work
  • The first approach would almost look similar to original JS code if you could use "export" syntax like in ES modules. Then at leas it would visually look similar to previous JS code that is using return in IIFE. Something like that:
//second approach
namespace app5.pages.admin {
    function privateFunc(){}
    function sendMail(email: string): void{
        privateFunc();
    }   
    export { sendMail } //Error: "Export declarations are not permitted in a namespace.",
}

But it's not supported. I guess it's not worth to change it now since namespaces are not used that often nowadays.

  • You basically can't represent namespaces that have same name as part of namespace. For example in JS I had `app.pages.app' namespace and I cannot have it in TS:
namespace app.pages.app {
    export const mailing = (function(){
        function privateFunc(){}
        function sendMail(email: string){
            app.pages.someOtherModule.foo(); //compiler error Property 'pages' does not exist on type 'typeof app'.
        }
        return {
            sendMail
        }
    })()    
}

In the end I had to use different name that I would use in TS files and use workaround to make old name work in other JS files:

(app.pages as any).app = app.pages.otherName;

Summary

As I mentioned it's all about easing migration of old JavaScript projects that use old namespace pattern (for example because they use ExtJS library) to TypeScript. I know that nowadays this pattern is not that popular because you should use ES modules. But I believe there are a lot of people that would love to move their projects to TS but it's hard because it would require to move to ES modules first. And it's a huge task itself. Actually my plan is to migrate current code to TS with old namespace and then try to migrate it to ES modules. It should be much easier to migrate when most of you code is typed. You have more confidence when compiler helps you.

If TS team thinks that it's only worth doing 1. proposal (just recognize what's recognized in allowJS now, without implementing JavaScriptNamespace alias proposal) it would be "good enough" for me because it will be much better than my current namespace workarounds.

But if I had this JavaScriptNamespace alias feature then it would be possible to include all my JS files to TS compilation with allowJS and this JS code will be available in TS! (of course function arguments will be any, but still). I would also get better IntellSense in current JS files because this namespace function will be recognized as creator of a namespace (when using Salsa).

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh
Copy link
Member

@sandersn thoughts? I feel like this is a pretty easy hack for detecting expando initializations

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Mar 11, 2020
@sandersn
Copy link
Member

How Hard Can It Be

Well, ThisType is recognised as special by the checker, not the binder. In particular, knowing that namespace is a function with an argument of type JavaScriptNamespace requires (1) binding+name resolution (2) parameter type resolution. The binder can fake (1) right now, but I don't relish adding fake support for (2).

Assuming that the binder could recognise uses of JavaScriptNamespace, supporting namespace declaration with it would be similar to support for Object.defineProperty.

Is It A Good Idea

I don't like the idea of introducing magic identifiers when there's any way around it. We introduced ThisType 3 years ago to accommodate dynamic JS frameworks that take an object literal with methods and process it such that the methods are eventually called with a different this. This was (and is) pretty common in the JS world, and at the time we thought of it as a necessary evil. Today, it's an annoying legacy evil. I'm not so sure that declaring namespaces this way is that common, especially as people have switched to modules in the last few years.

I think a good gauge of how desirable this feature is: how many code bases out there use it? How widely used are they? Scraping github for examples and relative popularity would help answer this question.

Other Upgrade Options

It's a long shot, but could you generate code to declare a namespace with a function inside and expand the function with expando assignments?

// generated code
declare namespace app.pages {
  declare function admin(): void
}
// original assignment
namespace("app.pages.admin");
app.pages.admin.mailing = (function () {
    return {
        sendMail: function (email: string): void { }
    }
})()

@mpawelski
Copy link
Author

Thank you guys for response!

I was hoping that TS support some kind of "ambient expando object in nested namespace", and @sandersn workaround provides it.

It just feels a bit hacky and would be great if Typescript support similar "expando" pattern on empty {} object so I could make this workaround a bit better like

declare namespace app.pages {
  const admin: {}
}
namespace("app.pages.admin");
app.pages.admin.mailing = (function () {
    return {
        sendMail: function (email: string): void { }
    }
})()

I also don't understand why it doesn't work with assigning just object literal (without wrapping in IIFE)

declare namespace app.pages {
    function admin(): void
  }
namespace("app.pages.admin");
app.pages.admin.mailing =  {
    sendMail: function (email: string): void { }
}

I got 'mailing' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer. Error.

But it's not a big problem. This workaround is good enough and resolves most of my problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants