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

--outFile & --module concatenation #5090

Merged
merged 36 commits into from
Nov 9, 2015

Conversation

weswigham
Copy link
Member

With this we support simple concatenation of amd and systemjs modules into the specified output file. ES6, UMD, and Commonjs all can't be bundled to via simple concatenation and path correction, so they are not a part of this change and it is an error to attempt to use one of them with --outFile.UMD was original on the list of formats to accept... but I realized that was silly when UMD contains support for commonjs (in addition to AMD), which we couldn't support - it seems silly to emit UMD when it only works in an AMD environment.

Since we use --out with --module commonjs internally (a lot), this changed the baselines for many tests.

This is, in effect, an alternative to #4754 which works for a handful of our module emits and doesn't require the user tell us an entrypoint.

@mhegazy care to give input?

@weswigham
Copy link
Member Author

@mhegazy Do you want to look at this at this point? I've got js and dts concatenation in.

@weswigham weswigham mentioned this pull request Oct 20, 2015
@@ -1756,6 +1796,26 @@ namespace ts {
};
}

export function makeModulePathSemiAbsolute(host: EmitHost, currentSourceFile: SourceFile, externalModuleName: string): string {
let quotationMark = externalModuleName.charAt(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

my only comment here, is we can simplify this using the resolver to get the symbol and then use the symbol's declaration filename to get the semiAbsolutePath,

here is what i had in mind.

        function emitExternalModuleSpecifier(moduleSpecifier: Expression) {
            Debug.assert(moduleSpecifier.kind === SyntaxKind.StringLiteral);

            if (compilerOptions.out) {
                let moduleSymbol = resolver.getSymbolAtLocation(moduleSpecifier);
                if (moduleSymbol && moduleSymbol.valueDeclaration &&
                    moduleSymbol.valueDeclaration.kind === SyntaxKind.SourceFile &&
                    !isDeclarationFile(<SourceFile>moduleSymbol.valueDeclaration)) {
                    let nonRelativeModuleName = getExternalModuleNameFromPath((<SourceFile>moduleSymbol.valueDeclaration).fileName);
                    write("\"");
                    write(nonRelativeModuleName);
                    write("\"");
                    return;
                }
            }

            writeTextOfNode(currentSourceFile, moduleSpecifier);
        }

        function getExternalModuleNameFromPath(fileName: string) {
            let sourceFilePath = getNormalizedAbsolutePath(fileName, host.getCurrentDirectory());
            let commonSourceDirectory = host.getCommonSourceDirectory();
            sourceFilePath = sourceFilePath.replace(commonSourceDirectory, "");
            return removeFileExtension(sourceFilePath);
        }

@mhegazy
Copy link
Contributor

mhegazy commented Nov 7, 2015

Once this goes in, please update the breaking change page: https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes

Add a snippit about it in What's new page: https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#typescript-18-upcoming

And update the documentation for compiler options in: https://github.com/Microsoft/TypeScript/wiki/Compiler-Options

@basarat
Copy link
Contributor

basarat commented Dec 10, 2015

Is this still only amd/systemjs? If so the statement in roadmap https://github.com/Microsoft/TypeScript/wiki/Roadmap#18 Concatenate module output with --outFile seems misleading.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 10, 2015

Is concatenating node/commonjs modules an interesting/common scenario?

@basarat
Copy link
Contributor

basarat commented Dec 10, 2015

Is concatenating node/commonjs modules an interesting/common scenario?

Not really. In all honesty just as interesting as amd, none of these bundlings are likely to happen in isolation i.e. module authors expect these modules to be consumed by users that will do bundling themselves, bundling the module code along with the app code. Was just curious. Thanks for the clarification 🌹

@hypno2000
Copy link

there is definately need for this in commonjs/amd: https://github.com/TypeStrong/dts-bundle

The main use-case is generating definition for npm/bower modules written in TypeScript (commonjs/amd) so the TypeScript code should following the external-module pattern (using import/export's and --outDir).

@mhegazy
Copy link
Contributor

mhegazy commented Apr 13, 2016

@hypno2000 this is covered by #4433

@wbhob
Copy link
Contributor

wbhob commented Apr 27, 2017

I don't understand; can we concatenate commonjs modules into a single file now, or are they still all segmented?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2017

This is a year old PR, I would say open a new issue instead.

And, no; common js modules can not be concatenated.

@weswigham weswigham deleted the out-module-concat branch August 17, 2017 23:04
@alexeagle
Copy link
Contributor

Hey @weswigham
With --outFile every AMD module is given a name. Is it possible to have the same thing when emitting to many files?

eg.
thing.ts

export function add(i: number, i2: number) {
    return i + i2;
}

compiled with --outFile
thing.js

define("thing", ["require", "exports"], function (require, exports) {
    "use strict";
    exports.__esModule = true;
    function add(i, i2) {
        return i + i2;
    }
    exports.add = add;
});

I want the same define("thing", ["require", "exports"]... without --outFile so I can concat the files together myself later in the build system. Is it possible?

@weswigham
Copy link
Member Author

@alexeagle Per our docs, you can add a ///<amd-module name="NamedModule"/> directive to the top of each file to name it.

@alexeagle
Copy link
Contributor

I don't want TypeScript authors to be aware of the module system used (this would be for karma, but for other bundlers I'd use ESM modules). Ideally I'd like the module names chosen the same way they are for --outFile
I could add the amd-module directive in a BeforeTransform I suppose.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants