Add options to wildcard #86

CKGrafico · 2018-01-10T14:32:18Z

Extend #77 (comment) PR
In progress adding @Toilal request

Toilal · 2018-01-10T14:35:29Z

In fact it should read tsconfig baseUrl and paths properties. In my use case, I have 2 wildcards ...

{
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "@/*": ["src/*"],
      "#/*": ["app/*"]
    },
   ...
  }
}

(Library template, source files lies in src/* and demo app files lies in app/*)

CKGrafico · 2018-01-10T14:37:01Z

Docs modified

CKGrafico · 2018-01-10T14:39:37Z

mm @Toilal this will need more work let me..

Toilal · 2018-01-10T14:40:27Z

If it's not possible, maybe the option could accept an array ?

Toilal · 2018-01-10T14:46:43Z

Would it be possible to loop on options.paths items matching .*/\* (regex) until if finds a file for the requested moduleName ?

CKGrafico · 2018-01-10T15:00:53Z

I have to change docs and two more minor things and it's done

johnnyreilly · 2018-01-10T15:07:18Z

Good work! Could you take a look at the failing build please?

CKGrafico · 2018-01-10T15:10:22Z

@johnnyreilly sorry I'm modifiyng my VueProgram.js and translating to your VueProgram.ts xDD

PS: mmm the test always the test haha give me some minutes

CKGrafico · 2018-01-10T15:57:31Z

Your tests are not working for me on Windows because absoluth path returns C:/... but I hope are ok now

johnnyreilly · 2018-01-10T16:09:00Z

For what it's worth, I'm a Windows guy but I use the Linux subsystem to run fork-ts-checker-webpack-plugin tests. @piotr-oles could you review this when you get a moment please?

CKGrafico · 2018-01-10T16:09:53Z

But can you do it on another PR xD? Is not related with my changes

johnnyreilly · 2018-01-10T16:46:12Z

Sorry - not sure I understand what you meant?

Either way - tests are passing now 😄

prograhammer · 2018-01-10T22:32:08Z

src/VueProgram.ts

-    const pattern = options.paths ? options.paths['@/*'] : undefined;
-    const substitution = pattern ? options.paths['@/*'][0].replace('*', '') : 'src';
-    const isWildcard = moduleName.substr(0, 2) === '@/';
+    const discartedSymbols = ['.', '..', '/'];


Is "discarted" meant to be "discarded" (misspelling)? 😄

prograhammer · 2018-01-10T22:36:55Z

README.md

@@ -210,8 +210,7 @@ import Hello from '@/components/hello.vue'
 }
 ```

-6. The commonly used `@` path wildcard will work if you set up a `baseUrl` and `paths` (in `compilerOptions`) to include `@/*`. If you don't set this, then 
-the fallback for the `@` wildcard will be `[tsconfig directory]/src` (we hope to make this more flexible on future releases):  
+6. It accepts any wildcard in your TypeScript configuration:  


Since we have this, the wildcard option in fork-ts-checker-webpack-plugin` was dropped right?

Yeah, it looks like you dropped that option (good!) since I don't see it here in the final file changes. 😛

My fault I was using Github "online" haha and every change was a commit

prograhammer · 2018-01-10T22:47:34Z

@CKGrafico Looks good!

But I see you are going down the route I took too (trying to cover for the common cases). Let's hope there are no more folks giving us future wildcard issues before we beg Microsoft for an API in TypeScript to resolve those paths in tsconfig.json (or we'll finally tackle it ourselves!). 😨

CKGrafico · 2018-01-11T09:12:47Z

Anybody needs something else?

prograhammer · 2018-01-11T19:56:24Z

Looks good to me to merge! 🚀

Toilal · 2018-01-11T20:26:57Z

I experience something I can't understand (again :p)... Even without this change, my project is working properly with # as alias (It just runs 0.3.0 release). I can catch TSLint errors on some Vue component imported with # symbol.

You can reproduce this by running, this should raise errors in src/app/Dummy.vue

git clone https://github.com/Toilal/vue-webpack-template.git -b test-#-import test-import
cd test-import
npm i
npm run build

Toilal · 2018-01-11T21:47:12Z

@CKGrafico Could you test with #87 (https://github.com/Toilal/fork-ts-checker-webpack-plugin/tree/vue-non-ts-module-simple) ?

It seems that we can handle wildcard module names the same way we handle relative module name (by building the "full" module name). TypeScript is able to solve paths like /home/toilal/project/test-import/src/components/#/components/Dummy.vue making manual building from tsconfig just useless.

prograhammer · 2018-01-11T23:05:23Z

@CKGrafico

I just ran your PR locally npm run test and I'm getting 2 fails. Anybody else getting this?

 [INTEGRATION] vue
    ✓ should create a Vue program config if vue=true (298ms)
    ✓ should not create a Vue program config if vue=false (239ms)
    ✓ should create a Vue program if vue=true (269ms)
    ✓ should not create a Vue program if vue=false (239ms)
    ✓ should get syntactic diagnostics from Vue program (254ms)
    1) should not find syntactic errors when checkSyntacticErrors is false
    2) should find syntactic errors when checkSyntacticErrors is true

CKGrafico · 2018-01-11T23:07:40Z

I'm going to check asap.
@prograhammer Travis is working well isn't it? I had problems running this on windows (and other PRs of this project) but I will doublecheck

Toilal · 2018-01-11T23:13:14Z

@prograhammer I had this issue too. If you are using yarn, run yarn upgrade. (that's why yarn.lock has changed on #87)

prograhammer · 2018-01-11T23:21:15Z

In the integration vue test, the example-wild.vue also needs a semantic error (this is good to have in the tests). So that line should be updated to:

public x: string = 1;

Then all passes. Weird that Travis didn't catch it?

prograhammer · 2018-01-11T23:23:57Z

So the total is 3 errors. 2 semantic, and 1 syntactic.

@CKGrafico if you want to update that line in example-wild.vue in the integration test to be a semantic error (just like example.vue) in this PR then I think all is good.

Toilal · 2018-01-11T23:25:01Z

I'm pretty sure you don't run same TypeScript or TSLint version than the travis build.

prograhammer · 2018-01-11T23:31:35Z

Ah @Toilal gotcha. Let me check that...

prograhammer · 2018-01-11T23:36:49Z

Or maybe webpack is suddenly silent? I'm on my phone and can't get to my laptop til later tonight to check.

CKGrafico · 2018-01-12T10:32:39Z

@Toilal not sure that your PR :/ will work to every SO
@prograhammer I'm testing this with yarn instead of npm and updating yarn.lock asap, sorry for forget yarn I'm not using it since package-lock.json exist

CKGrafico · 2018-01-12T10:34:39Z

Checked if you don't use windows npm test is working well (The windows errors is not related with this PR we can merge this and made another one)

Toilal · 2018-01-12T10:51:27Z

I'll close my PR, it's simpler but only works magically in my project, with yours we are pretty sure it should work everywhere. Thanks a lot, it'll be included on my webpack template fork https://github.com/Toilal/vue-webpack-template when it's released in this library.

CKGrafico · 2018-01-12T12:01:19Z

Test failing after I've updated yarn? amazing

CKGrafico · 2018-01-12T12:09:13Z

Ok in index.spec something like this

console.log(stats.compilation.errors, stats.compilation.errors.length)

returns an array of '1' is not number and length 1 and you're expecting to be 1

but on vue.spec you have the same and you are expecting 2, I'm assuming that is a strange mistake? and correcting it

prograhammer · 2018-01-12T18:15:06Z

Yeah I think there might have been some extra Webpack output there, but with the latest package.json lock it seems to have gone away. (BTW, you guys should update to latest Node 8 (now LTS) and NPM 5 because NPM is as fast as Yarn now (and yarn is just another abstraction to learn on top of NPM anyway).

Anyways, all good to merge! 🚀

Toilal · 2018-01-12T18:56:06Z

From my experience it's still a bit slower. In fact. when there's a package-lock file, I use npm5, else I use Yarn.

CKGrafico · 2018-01-12T21:34:37Z

I think the PR is ready now :)

johnnyreilly · 2018-01-12T22:28:48Z

Cool - @piotr-oles could you review when you get a moment please?

Toilal · 2018-01-16T15:32:54Z

Could you have a look to #88 ? Not sure how it's related ...

CKGrafico · 2018-01-16T17:47:27Z

Please is you're not to merge this soon, tell us and we will made a fork for our projects :D

johnnyreilly · 2018-01-16T18:35:04Z

@piotr-oles could you take a look when you get a moment? @CKGrafico - until that time would you like me to publish it to our makeshift @next channel? https://github.com/TypeStrong/fork-ts-checker-webpack-plugin

CKGrafico · 2018-01-16T18:49:43Z

Not the best but sounds like a solution 💃

johnnyreilly · 2018-01-16T20:23:29Z

Give me an hour...

johnnyreilly · 2018-01-16T20:33:16Z

Take a look: https://github.com/typestrong/fork-ts-checker-webpack-plugin

CKGrafico · 2018-01-16T22:46:10Z

@johnnyreilly mmm I think that you're not using my PR xD CKGrafico@f9c01f9 --> d6ce069#diff-23d36b6fe597b409b93d20e36766ff64R31

johnnyreilly · 2018-01-17T06:24:55Z

Yup - I didn't build from the patch-1 branch. Doing that now.

johnnyreilly · 2018-01-17T06:38:29Z

Is this better? 4fce3bd

piotr-oles · 2018-01-31T16:49:14Z

Thank you for your work and sorry for delay :)

Add options to wildcard

5c1e808

Update README.md

909ddf8

Update VueProgram.ts

e705892

Fix CI

f0bf093

CKGrafico added 3 commits January 10, 2018 16:15

Update README.md

d1156a0

Modify tests

507d525

Fix vueprogram

f9c01f9

prograhammer reviewed Jan 10, 2018

View reviewed changes

Learn english Quique

e42b3e1

Toilal mentioned this pull request Jan 11, 2018

Resolve all non TS module with simple concatenation (vue) #87

Closed

Update yarn.lock

ee28476

Modify vue spect to be tha same as index spec

7adf65e

CKGrafico mentioned this pull request Jan 17, 2018

Pr/ts lint pwa-builder/PWABuilder#268

Merged

wonderful-panda mentioned this pull request Jan 31, 2018

Improve module resolution of *.vue #94

Merged

piotr-oles approved these changes Jan 31, 2018

View reviewed changes

piotr-oles merged commit 840d0fd into TypeStrong:master Jan 31, 2018

Uh oh!

Add options to wildcard #86

Add options to wildcard #86

Uh oh!

Conversation

CKGrafico commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Toilal commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CKGrafico commented Jan 10, 2018

Uh oh!

CKGrafico commented Jan 10, 2018

Uh oh!

Toilal commented Jan 10, 2018

Uh oh!

Toilal commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CKGrafico commented Jan 10, 2018

Uh oh!

johnnyreilly commented Jan 10, 2018

Uh oh!

CKGrafico commented Jan 10, 2018

Uh oh!

CKGrafico commented Jan 10, 2018

Uh oh!

johnnyreilly commented Jan 10, 2018

Uh oh!

CKGrafico commented Jan 10, 2018

Uh oh!

johnnyreilly commented Jan 10, 2018

Uh oh!

prograhammer Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

CKGrafico Jan 11, 2018

Choose a reason for hiding this comment

Uh oh!

prograhammer Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

prograhammer Jan 10, 2018

Choose a reason for hiding this comment

Uh oh!

CKGrafico Jan 11, 2018

Choose a reason for hiding this comment

Uh oh!

prograhammer commented Jan 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CKGrafico commented Jan 11, 2018

Uh oh!

prograhammer commented Jan 11, 2018

Uh oh!

Toilal commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Toilal commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prograhammer commented Jan 11, 2018

Uh oh!

CKGrafico commented Jan 11, 2018

Uh oh!

Toilal commented Jan 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prograhammer commented Jan 11, 2018

Uh oh!

prograhammer commented Jan 11, 2018

Uh oh!

Toilal commented Jan 11, 2018

Uh oh!

prograhammer commented Jan 11, 2018

Uh oh!

prograhammer commented Jan 11, 2018

Uh oh!

CKGrafico commented Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

CKGrafico commented Jan 10, 2018 •

edited

Loading

Toilal commented Jan 10, 2018 •

edited

Loading

Toilal commented Jan 10, 2018 •

edited

Loading

prograhammer commented Jan 10, 2018 •

edited

Loading

Toilal commented Jan 11, 2018 •

edited

Loading

Toilal commented Jan 11, 2018 •

edited

Loading

Toilal commented Jan 11, 2018 •

edited

Loading

CKGrafico commented Jan 12, 2018 •

edited

Loading

prograhammer commented Jan 12, 2018 •

edited

Loading

johnnyreilly commented Jan 17, 2018 •

edited

Loading