search

aurelia / webpack-plugin

A plugin for webpack that enables bundling Aurelia applications.
MIT License
90 stars 36 forks source link

Typings not being resolved for webpack.config.ts #143

Open fkleuver opened 6 years ago

fkleuver commented 6 years ago

When configuring webpack with a webpack.config.ts rather than a webpack.config.js, the typings are not properly resolved:

image

Everything works, it's just this error and the lack of typings/intellisense that makes the experience suboptimal.

I wouldn't mind submitting a PR if this is indeed a "bug", but I just want to make sure first that there's not some kind of setting that I'm missing.

Alexander-Taran commented 6 years ago

try adding "types":"dist/types/index.d.ts" to plugins package.json first http://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html if it will work.. it will be a small PR (-:

fkleuver commented 6 years ago

Bulls-eye, that did the trick! A simple PR for a change :-) Cheers!

jods4 commented 6 years ago

Typings were removed because of #118

I keep this issue open as a reminder to add them back, but it is unlikely to happen before s-panferov/awesome-typescript-loader#484 is fixed.

fkleuver commented 6 years ago

@jods4 I switched to ts-loader because at-loader was giving me a variety of issues integrating with non-default stuff. #118 just goes to show that it's indeed a quirky tool.

In the end I noticed no difference in speed. It seems ts-loader has caught up (learned their lesson, maybe?).

Regardless of that, I'm not sure why removing the typings was the better solution here. As far as I know any typing errors caused by external dependencies can be suppressed with skipLibCheck: true. I have this option on by default because I couldn't care less about the absolute correctness of external lib typings as long as they serve documentation needs (they're too often conflicting anyway).

That's an easy workaround whereas there is (apparently?) no workaround for getting the typings included when they're absent.

jods4 commented 6 years ago

Forcing skipLibCheck on users is bad.

These definitions apply to your build file, which is less important than your full app.

The bug with at-loader is that those definitions somehow get included into the full app build, although nothing in app code references aurelia-webpack-plugin.

There are workarounds on the build side as well:

fkleuver commented 6 years ago

Forcing skipLibCheck on users is bad.

These definitions apply to your build file, which is less important than your full app.

webpack.config.ts is compiled by ts-node and the tsconfig.json for that is usually separate anyway. You're only forcing it on that specific config. The "bad" stays on the build side.

The bug with at-loader is that those definitions somehow get included into the full app build, although nothing in app code references aurelia-webpack-plugin.

I tested it just now and awesome-typescript-loader works just fine with types added to package.json and skipLibCheck: false. Maybe it's been resolved?

/// should work?

That gives me this error: [ts] Cannot find type definition file for './node_modules/aurelia-webpack-plugin/dist/types/index.d.ts'.

Using path instead of types doesn't work either; it simply has no effect (I did reload after changing).

You can copy the .d.ts file into your build folder.

I suppose, but that feels worse than having to set skipLibCheck: true for the ts-node specific tsconfig. There's already so much involved with getting everything type-checked, rather not have to manually keep an external .d.ts in sync on top of that..

jods4 commented 6 years ago

I don't like this situation either.

The top priority is that starting a new app with Webpack should "just work". At the time, at-loader was popular (I think it even was in some skeletons/templates, maybe even our own) and that scenario would fail.

Fewer people write their build in TS so this is comparatively less important. It's important that you can make it work, but if there has to be a workaround it'll be on the build-side, not the app-side.

I'm open to any suggestion or new idea that would make the situation better.

I tested it just now and awesome-typescript-loader works just fine with types added to package.json and skipLibCheck: false. Maybe it's been resolved?

Maybe it's been resolved. If we can confirm that the problem was fixed I support bringing back the types in package.json 100%.

fkleuver commented 6 years ago

I tested it with a clean au new with typescript+webpack, and I understand now why I didn't get the issue in my own project.

The tsconfig.json is the real problem. All aurelia ts projects and skeletons seem to have "filesGlob" even though that's not supported. Maybe it once was (and that would be way back), but pretty sure it's simply ignored now.

So this tsconfig:

  "exclude": [
    "node_modules",
    "aurelia_project"
  ],
  "filesGlob": [
    "./src/**/*.ts",
    "./test/**/*.ts",
    "./custom_typings/**/*.d.ts"
  ],

Is essentially saying "include everything except aurelia_project and node_modules".

That includes top-level project files such as webpack.config.ts.

Change it either to this:

  "exclude": [
    "node_modules",
    "aurelia_project",
    "./*.ts"
  ]

Or to this:

  "include": [
    "./src/**/*.ts",
    "./test/**/*.ts"
  ]

And webpack.config.ts is no longer included -> error is gone.

For clarification, there's 3 properties pertaining to input: "include", "exclude" and "files"

In all my projects I only ever specify "include". I think this is the cleanest solution. Only ["src"] for the build, and ["src", "test"] for testing. That's all you need in 99% of cases.

I sort of knew this all along but I figured there must have been something I didn't know about "filesGlob". I triple checked the docs now, and I'm 100% sure it must be replaced with "include" in all aurelia projects (and checked for correctness) if we want everything to be consistent on the long term.

Alexander-Taran commented 6 years ago

@fkleuver that will result in tests being compiled and given to webpack.. always gives me headaches.. I also like them to be next to models as *.test.ts for at-loader there could be files:[] but it errors out for ts-loader.. and seems to have strange sideeffects..

nice catch though. We should figure out tsconfig(s) for every cli/skeleton check them with both loaders.. and all cli included testers..

then we can proceed. I think

fkleuver commented 6 years ago

@Alexander-Taran That's what's currently happening anyway. The example I gave changes as little as possible to the existing configuration. Removing "tests" from the glob might break stuff if we're having an "include" to begin with.

We should figure out tsconfig(s) for every cli/skeleton check them with both loaders.. and all cli included testers..

It should be quite simple as they're all very similar. I just learned that "filesGlob" is specific to the atom editor. It was never part of typescript itself.

So to non-atom users it never did anything. "includes" will behave the way "filesGlob" was intended to.

For atom users, nothing should change since "includes" does the same thing in typescript as "filesGlob" does in atom.

This would also eliminate inconsistencies between non-atom users and atom users (@EisenbergEffect ? ;)) in how things are included/compiled.

EisenbergEffect commented 6 years ago

I agree. We want this to be as consistent as possible. @AshleyGrant can you look into updated the skeletons based on @fkleuver's PR to the CLI?

thomas-darling commented 5 years ago

Can we please either include the typings for aurelia-webpack-plugin in the package, or alternatively publish them as @types/aurelia-webpack-plugin?

Missing typings really get in the way, and having to work around it by adding your own .d.ts file is pretty annoying - especially when its the last dependency in your project that needs one of those ;-)