Add packages with prefix vsf- to webpack process

[filrak]

What is the motivation for adding / enhancing this feature?

Currently none of the third party libs are processed through loaders in our webpack process. While it's ok for most of them it's not ok for 3rd party VS modules. They should he included into our webpack process.

This requires changing regular expressions for loader in core/build folder.

What are the acceptance criteria

• node_modules with name starting from vsf- should be included into our webpack process for all loaders (as it would be in in the core)

Can you complete this feature request by yourself?

NO

Additional information

Feel free to ask for details on slack

[DaanKouters]

@filrak i'm quiet busy with this, but can't make any progress.

What i did so far:
In core/build/webpack.base.config.ts
i changed /node_modules/ in the excludes to /node_modules\/(?!vsf-)/

I added my test npm package path to the babel loader rule include property.
path.resolve(__dirname, '../../node_modules/vsf-layered-navigaton'),

changed the ts-loader rule to:
{ test: /\.ts$/, loader: 'ts-loader', options: { appendTsSuffixTo: [/\.vue$/], allowTsInNodeModules: true }, exclude: /node_modules\/(?!vsf-)/ }

Added the option allowTsInNodeModules and set to true.

In tsconfig.json
i added my testpackage in the include property: "node_modules/vsf-layered-navigation/**/*.ts" (for testing purposes)

====================

This way the compiling in the console went sucessfully. But when you call the vsf instance in browser, you see that the npm package seems not to be processed through babel (f.e. console fills with not tranformed imports from my test npm package)

I'm really stuck with this and as you mentioned before that it would be an easy implementation i feel like i'm overdoing something right now. I can really use some help with this one from someone with more webpack config experience.

[filrak]

Hey! Please play with include property of webpack loaders for babel-lowder. You're close to the end I guess and the thing that is lacking is this property

[DaanKouters]

hi @filrak :)
ok, i'll play around with it, but please be very critical in the Pull Request.
This has to be done right :)

[zimme]

I'd like to request support for scoped packages as well i.e. @kodbruket/vsf-storyblok-module

[DaanKouters]

@zimme great job.

If i add your changes to core/build/webpack.base.config.ts and add the @getnoticed/vsf-layered-navigation NPM package i get compilation error:

Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for 'node_modules/@getnoticed/vsf-layered-navigation/index.ts'

Also, when you visit the VSF instance, you get 'SyntaxError: Unexpected token import'. This error comes from: 'node_modules/ts-node/src/index.ts:431:14'

When Googling this issue it seems that we need some changes to tsconfig.json, but to be honoust i did not find the solution so far. Any help is welcome

[zimme]

I also ran into this problem when testing today. As you say, I believe tsconfig might need updating as it excludes node_modules.

[zimme]

@DaanKouters Could you try with the changes added in this commit and see if it works?

[DaanKouters]

hi @zimme i tested it with your changes, at first i still got the compile error, but i noticed you left "node_modules" in the exclude property.
When i remove that, compilation runs just fine.
So now the exclude property of tsconfig.json looks like:

"exclude": [
    "!node_modules/vsf-*/**/*.ts",
    "!node_modules/@*/vsf-*/**/*.ts",
    "**/*.spec.ts"
  ]

But when i go to the storefront in browser, the console throws multiple errors on 'vuestorefront/node_modules/ts-node/src/index.ts:431:14'.

This message led me to this issue on the ts-node github:
TypeStrong/ts-node#621

I don't have a solution for this.

Maybe valuable to know i added a NPM package to my test project this way:
yarn add @getnoticed/vsf-layered-navigation -W - as suggested by Yarn. maybe this is the point where it goes with improper linking? I don't know.

[zimme]

@DaanKouters Try leaving node_modules as it was in tsconfig.json

and add these changes to tsconfig-build.json

"exclude": [
    "**/*.spec.ts",
    "!node_modules/vsf-*/**/*.ts",
    "!node_modules/@*/vsf-*/**/*.ts"
  ]
]

ps. You're adding the package in the way I expect it to be added so that's fine.

[DaanKouters]

@zimme ok, when i leave the exclude as it was (with "node_modules") i get in compilation:
Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for

And adding the vsf paths to the exclude in tsconfig-build didn't make a noticable change, still the ts-node error 'node_modules/ts-node/src/index.ts:431:14'

[zimme]

I saw that the regex to exclude everything except vsf- packages were wrong and I looked into ts-node and realized it didn't use the include/exclude from tsconfig so I'm testing setting an environment variable to ignore all but vsf- packages in node_modules which ignores the whole folder by default

[zimme]

Ok, so I've been trying out different things here and I've been able to overcome the Error: TypeScript emitted no output for error and are now getting TypeError: Unable to require ".d.ts" file. instead.

I've added TS_NODE_IGNORE=false to make sure ts-node isn't ignoring things in node_modules and I've made sure the tsconfig files isn't excluding any node_modules related things and I can't get past this .d.ts require problem when it comes to typescript code in node_modules.

I've tried with preserveSymlinks to make sure it's not a yarn/lerna symlinked packages related issue either.

It seems that all build tools around typescript have a default to ignore anything node_modules related to force people to actually build typescript into javascript before publishing and undoing it is harder than I had hoped.

So unless someone smarter than me can figure this one out I believe we should revert #2875 and ask module developers to build their typescript code before publishing their packages.

What could be done to ease that is to provide the build script needed via the vue-storefront-cli tool so that module developer don't have to configure their own build pipeline but just npm install the cli and add it to the script section of their package.json file.

/cc @filrak @DaanKouters

[zimme]

This issue should be re-opened.

[DaanKouters]

Ok, so I've been trying out different things here and I've been able to overcome the Error: TypeScript emitted no output for error and are now getting TypeError: Unable to require ".d.ts" file. instead.

I've added TS_NODE_IGNORE=false to make sure ts-node isn't ignoring things in node_modules and I've made sure the tsconfig files isn't excluding any node_modules related things and I can't get past this .d.ts require problem when it comes to typescript code in node_modules.

I've tried with preserveSymlinks to make sure it's not a yarn/lerna symlinked packages related issue either.

It seems that all build tools around typescript have a default to ignore anything node_modules related to force people to actually build typescript into javascript before publishing and undoing it is harder than I had hoped.

So unless someone smarter than me can figure this one out I believe we should revert #2875 and ask module developers to build their typescript code before publishing their packages.

What could be done to ease that is to provide the build script needed via the vue-storefront-cli tool so that module developer don't have to configure their own build pipeline but just npm install the cli and add it to the script section of their package.json file.

/cc @filrak @DaanKouters

hi @zimme firstly, thanks for your efforts
As i was searching for solutions i often came across it was a better practice to let the packages be prebuilt the typescript like you also say above.

This is going a bit above my hat right now, so if i can help with testing or something please let me know

[jahvi]

@zimme How did you manage to overcome the TypeScript emitted no output for .../index.ts issue? I've been trying to give this a go but I haven't been able to get past that issue.

[zimme]

@jahvi The changes in the last 3 commits here is what I did.
https://github.com/zimme/vue-storefront/commits/zimme/process-vsf-packages-with-webpack

I also don't remember if I had the npm package with the vsf module as a dependency in vue-storefront or in the theme

I'm not sure that the tsconfig-paths is necessary.

[jahvi]

I haven't had any luck with this unfortunately, I also wonder if the best approach here would be to make it easier for 3rd parties to compile their TS code before publishing modules/extensions, maybe by providing a boilerplate or step by step instructions.

Maybe someone with more experience with TS could help.

[zimme]

@patzick Did you solve the problem we ran into here before adding back the commit?

[patzick]

@zimme sorry this was caused by merge into master. I'm reopening it.

[bloodf]

This issue has been closed due to inactivity.

If you want to re-open the issue, please re-create the issue, and post newer information on this problem.