Add custom transformers option #589

layershifter · 2018-08-22T16:14:50Z

Fixes #502.

Adds functionality to use custom transformers: https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API

Available via getCustomTransformers() option, the idea taken from awesome-typescript-loader.

layershifter · 2018-09-03T10:35:31Z

@ivogabe can you please review?

ivogabe · 2018-09-11T15:39:16Z

Thanks @layershifter for the PR! Sorry for the delay, I've been busy with other projects and didn't have time to take a look at this yet. Overall the changes look good, though I was wondering whether it is a large limitation that we don't pass a ts.Program to getCustomTransformers? That seems to be the difference with awesome-typescript-loader. Correct me if I'm wrong, but I assume that we cannot pass the project to the getCustomTransformers as we don't have one when isolatedModules is enabled, as we are then using a file-based compiler.

An alternative would be to change the type to (string | ((program?: ts.Program) => ts.CustomTransformers | undefined)), and only pass the program if isolatedModules is disabled. I assume that you would need the ts.Program if you need some type info of the program and we simply don't have that with isolatedModules enabled. If you need that type info, you should just not use isolatedModules. What do you think about that? If you want to make the change, you should call getCustomTransformers in the FileCompiler and ProjectCompiler and not in checkAndNormalizeSettings.

ivogabe · 2018-09-11T15:39:34Z

gulpfile.js

@@ -149,6 +149,7 @@ gulp.task('test', gulp.series('test-run', function testVerify() {
 	let failed = false;
 	function onError(error) {
 		failed = true;
+		console.log(error)


Could you add a semicolon here?

I will delete this line, it should not be there

ivogabe · 2018-09-11T15:40:08Z

lib/main.ts

@@ -33,6 +33,32 @@ function compile(param?: any, theReporter?: _reporter.Reporter): compile.Compile
 	return proj(theReporter);
 }

+function getFinalTransformers(getCustomTransformers?: GetCustomTransformers): FinalTransformers {
+	if (typeof getCustomTransformers === 'function') {
+		return getCustomTransformers


Yep, seems we need tslint there :)

ivogabe · 2018-09-11T15:40:17Z

lib/main.ts

+
+	if (typeof getCustomTransformers === 'string') {
+		try {
+			getCustomTransformers = require(getCustomTransformers)


ivogabe · 2018-09-11T15:40:32Z

lib/main.ts

+		} catch (err) {
+			throw new Error(
+				`Failed to load customTransformers from "${getCustomTransformers}": ${err.message}`
+			)


ivogabe · 2018-09-11T15:40:38Z

lib/main.ts

+		if (typeof getCustomTransformers !== 'function') {
+			throw new Error(
+				`Custom transformers in "${getCustomTransformers}" should export a function, got ${typeof getCustomTransformers}`
+			)


ivogabe · 2018-09-11T15:40:50Z

lib/main.ts

+		return getCustomTransformers
+	}
+
+	return null


ivogabe · 2018-09-11T15:43:48Z

lib/main.ts

@@ -46,7 +72,7 @@ function getTypeScript(typescript: typeof ts) {
 }

 function checkAndNormalizeSettings(settings: compile.Settings): compile.Settings {
-	const { declarationFiles, noExternalResolve, sortOutput, typescript, ...standardSettings } = settings;
+	const { customTransformers, declarationFiles, noExternalResolve, sortOutput, typescript, ...standardSettings } = settings;


Should this be getCustomTransformers?

It seems that you don't use the variable here, but we should keep it here as it will remove the property from standardSettings. That way we don't need the delete settings.getCustomTransformers; that you have later on.

ivogabe · 2018-09-11T15:45:03Z

lib/main.ts

@@ -166,6 +194,9 @@ module compile {
 				settings = fileNameOrSettings || {};
 			}

+			finalTransformers = getFinalTransformers(settings.getCustomTransformers);
+			delete settings.getCustomTransformers;


Using delete here will cause a performance problem, we used it before for a similar reason as here. The field can better be removed in checkAndNormalizeSettings, see my comment there.

ivogabe · 2018-09-11T15:45:27Z

readme.md

+```js
+getCustomTransformers: () => ({
+    before: [
+        styledComponentsTransformer(),


Could you also add the require call to be complete?

ivogabe · 2018-09-11T15:46:32Z

test/customTransformers/gulptask.js

+module.exports = function(newTS, lib, output, reporter) {
+	var getCustomTransformers = function () {
+		return {
+			before: [transformer],


Add a comment what the transformer does (eg remove all code)

layershifter · 2018-09-11T15:47:07Z

@ivogabe thanks for feedback!

Correct me if I'm wrong

Nope, you're fully right.

(string | ((program?: ts.Program) => ts.CustomTransformers | undefined))

Looks good to me because I have already thought about this 👍 I will make these changes tomorrow.

ivogabe · 2018-09-11T16:51:03Z

Great! Let me know if you run into any problems.

…nto custom-transformers # Conflicts: # lib/compiler.ts

layershifter · 2018-09-13T11:33:46Z

@ivogabe it seems that I made all requested changes. Can you please take a look? 😄

ivogabe · 2018-09-22T13:32:14Z

Thanks Alexander for your work! Changes look good. It will be part of the next release.

Yugloocamai · 2019-11-06T00:20:57Z

I am unable to get this feature working following the example in the docs :( The transformer is never called, even though it's imported successfully.

add getCustomTransformers()

e2907e8

layershifter force-pushed the custom-transformers branch from 83732b0 to e2907e8 Compare September 3, 2018 10:20

layershifter changed the title ~~[WIP] Add custom transformers~~ Add custom transformers option Sep 3, 2018

layershifter mentioned this pull request Sep 3, 2018

RFC: generate PropTypes, handledProps microsoft/fluent-ui-react#107

Closed

ivogabe requested changes Sep 11, 2018

View reviewed changes

Oleksandr Fediashov added 4 commits September 13, 2018 13:55

add require to readme

8ff209c

fix comments

4a59a95

Merge branch 'master' of https://github.com/ivogabe/gulp-typescript i…

06e5408

…nto custom-transformers # Conflicts: # lib/compiler.ts

more semi

5e4b041

ivogabe approved these changes Sep 22, 2018

View reviewed changes

ivogabe merged commit d78c372 into ivogabe:master Sep 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom transformers option #589

Add custom transformers option #589

layershifter commented Aug 22, 2018 •

edited

Loading

layershifter commented Sep 3, 2018

ivogabe commented Sep 11, 2018

ivogabe Sep 11, 2018

layershifter Sep 11, 2018

ivogabe Sep 11, 2018

layershifter Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

ivogabe Sep 11, 2018

layershifter commented Sep 11, 2018

ivogabe commented Sep 11, 2018

layershifter commented Sep 13, 2018

ivogabe commented Sep 22, 2018

Yugloocamai commented Nov 6, 2019 •

edited

Loading

Add custom transformers option #589

Add custom transformers option #589

Conversation

layershifter commented Aug 22, 2018 • edited Loading

layershifter commented Sep 3, 2018

ivogabe commented Sep 11, 2018

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

layershifter commented Sep 11, 2018

ivogabe commented Sep 11, 2018

layershifter commented Sep 13, 2018

ivogabe commented Sep 22, 2018

Yugloocamai commented Nov 6, 2019 • edited Loading

layershifter commented Aug 22, 2018 •

edited

Loading

Yugloocamai commented Nov 6, 2019 •

edited

Loading