Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Allow webpack configs to be authored in typescript (fixes #1241) - #1301

Closed
AdamWillden wants to merge 1 commit into
aspnet:devfrom
AdamWillden:typescript-webpack-config
Closed

AdamWillden wants to merge 1 commit into
aspnet:devfrom
AdamWillden:typescript-webpack-config

Conversation

@AdamWillden

@AdamWillden AdamWillden commented Sep 27, 2017

Copy link
Copy Markdown

@SteveSandersonMS good to make your acquaintance again (a nod from my KnockoutJS days, although you won't remember me)

This fix is entirely untested and simply following your instructions so we can get the ball rolling. Is this really all that's needed for the typescript to get parsed on demand??

+    if (options.useTypescriptConfig) {
+      require('ts-node/register');
+    }
+

I'll try and see if I can build the solution, replace the DLL and node_modules in my project and confirm the fix.

@AdamWillden

Copy link
Copy Markdown
Author

I can't seem to get the solution to build, are there any instructions I can follow? I'm using VS2017

@AdamWillden

AdamWillden commented Sep 27, 2017

Copy link
Copy Markdown
Author

Okay, so I can test the theory myself and am in the process of doing so.

Assuming the c# side works okay it's the TS we need to consider. Thus I'm using the following middleware options to mock the c# UseTypeScriptConfig option:

new WebpackDevMiddlewareOptions
{
    ConfigFile = "webpack.config.ts"
}

I have modified my node_modules copy of aspnet-webpack to mock the fix by sticking require('ts-node/register') in the same place.

By running this I've now found that using Typescript we define the config and use: export default which produces something different to the module.exports as such I need to 'detect' this.

A bit of logging has revealed that we receive the following object { default: [Function: config] }. I need to figure out the best way of detecting this...

Having come across this issue microsoft/TypeScript#2719 I have tested and found that console.log(webpackConfigExport.__esModule); results in Microsoft.AspNetCore.NodeServices:Information: true. So I can use that...

@AdamWillden

Copy link
Copy Markdown
Author

Right, yes, my imitation of this fix suggests it should work :-)

@michaelvolz michaelvolz left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I tested those changes and they seem to work fine.

@buvinghausen

Copy link
Copy Markdown

Can we please get this PR merged in? I really would like to jettison the javascript webpack config.

@michaelvolz

Copy link
Copy Markdown

I saw an update but these changes were still missing

@buvinghausen

Copy link
Copy Markdown

Why is getting user contributions in on this project so challenging?

@ghost ghost removed the cla-signed label Nov 14, 2017
@ghost ghost deleted a comment from dnfclas Nov 14, 2017
@ghost ghost deleted a comment from dnfclas Nov 14, 2017
@AdamWillden

Copy link
Copy Markdown
Author

@SteveSandersonMS I've resolved the conflicts created by the fix for #1378 - can you get this merged or reviewed so I don't have to maintain it please.

@SteveSandersonMS

SteveSandersonMS commented Nov 15, 2017

Copy link
Copy Markdown
Member

Thanks for this @AdamWillden. I'm planning to make a change to how this is implemented so that people can get the new functionality just from an updated NPM package without also needing an updated NuGet package (in other words, by implementing it without any C# changes). This is because we can ship updates to the NPM package almost any time, whereas NuGet packages only ship when ASP.NET itself ships. So, please don't feel compelled to maintain this PR in any way in the meantime.

@AdamWillden

Copy link
Copy Markdown
Author

@SteveSandersonMS works for me considering we'll get the update sooner that way. Thanks for the response

@AmyJeanes

AmyJeanes commented Nov 19, 2017

Copy link
Copy Markdown

I've been working around this using a file called webpack.dev.js:

require('ts-node/register')
module.exports = require("./webpack.config.ts");

And then using

new WebpackDevMiddlewareOptions
{
    ConfigFile = "webpack.dev.js"
}

But to be able to use them natively would be great

@SteveSandersonMS

SteveSandersonMS commented Jan 25, 2018

Copy link
Copy Markdown
Member

This is now implemented in a way that doesn't require new .NET packages: b2373e1

@AmyJeanes

Copy link
Copy Markdown

Very nice, what version is/will this change be in?

@SteveSandersonMS

SteveSandersonMS commented Jan 25, 2018

Copy link
Copy Markdown
Member

2.0.3, hopefully today

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.

6 participants