[WIP] Remove custom "npm link" behaviour

[roschaefer]

There is already a procedure for both npm and yarn to read an npm module
from a local folder. It's convenient and reliable.

E.g. yarn run dev:styleguide maybe works for in the browser. But if I
import @humanconnection/styleguide in the test, I get
non-deterministic behaviour.

Let's get rid of yarn run dev:styleguide

@appinteractive what is the command that you run before you upload to @humanconnection/styleguide? If I do yarn run build:lib, link the npm package in Nitro-Web I get the following error message:

 WARN  in ./plugins/styleguide.js                                                                                                   friendly-errors 02:08:15

"export 'default' (imported as 'Styleguide') was not found in '@human-connection/styleguide'        

The dropdown menu for Report contribution is neither visible nor clickable.

[appinteractive]

@roschaefer what is you issue here? Do we still have hot reload with that? I don’t think so, will test it. The npm link may be a solution but also a problem as you always will have to unlink it again which is tedious.

[appinteractive]

Your issue might come from the wrong command inside the styleguide. You have to run yarn dev, not yarn build:lib like you did. It’s described in the readme.

[appinteractive]

Needs more investigation if that implementation preserves the intended behavior.

[roschaefer]

@appinteractive it does not support hot reload. I find it perfectly acceptable to run yarn run something inside the styleguide before trying out changes in Nitro-Web.

The desire to test the styleguide from within another application shows a dependency issue: Test + Try-out components in the styleguide, don't use another application for it. If you need sources from another folder to test basic functionality of base components, these sources are in the wrong location, they should be in the styleguide folder.

[roschaefer]

@appinteractive if I have to run yarn run dev what commands need to be run before you upload to npm?

To reproduce:

1. git reset --hard && git clean -dfx
2. yarn install
3. Then what?

There is no .travis.yml where I would expect the exact steps.

[roschaefer]

With yarn run dev I'm still getting the same error message as above.

friendly-errors 12:12:25  WARN  in ./plugins/styleguide.js

friendly-errors 12:12:25 "export 'default' (imported as 'Styleguide') was not found in '@human-connection/styleguide'
friendly-errors 12:12:25
12:12:25 ℹ Waiting for file changes

12:12:25  READY  Server listening on http://127.0.0.1:3000

[appinteractive]

@appinteractive if I have to run yarn run dev what commands need to be run before you upload to npm? ...

@roschaefer there is a .travis.yml in the styleguide as the publishing happens through Travis. You may need to also run yarn build once.

But I think you don’t get the point here as you do not have to use it in the end. I find it irritating when functionality is ripped out because you can save a couple lines of code. I don’t get the point here. As far as I can see, you do not provide any viable solution or workable alternative here.

[roschaefer]

Well, the viable alternative is to move the required source code to Nitro-Styleguide. When I run yarn run dev I get this lovely playground. Why we don't write code in the demo template to try out the behaviour in the playground?

If we keep the removed code in this PR this creates a lot of confusion because you might a) see the changes in the browser in development mode but b) don't see a change if you write a test that depends on behaviour of the styleguide. I had that issue and was able to resolve it. But for our contributors it creates complexity and frustration.

[appinteractive]

We can encapsulate is hat code inside the plugin and reference it where inside the config. So there will be no confusion. Also does that not effect testing, it’s for getting the result of styleguide changes in a live feedback loop.

If it does interfere with testing, I agree to not recommend that route as it’s not intended for anyway but for styling and more rare tasks. But still, I will need that behavior the next weeks while adjusting styling etc. therefor I do not agree with ripping it out.

We should point them to the npm link method for casual use as it will be suitable for the most usecases. Would that be in your interest?

[roschaefer]

@appinteractive sounds like a plan! I'm OK to keep this code for now. We should write tests in the styleguide repository and find a way to try out the components interactively.

This PR can stay opened and we merge it + remove the code eventually.

[roschaefer]

@appinteractive by the way I would suggest that you try out yarn link at least once. Reason: When I install the new styleguide version, everything works as expected, but when I link the styleguide repository and run yarn run build:lib as stated in .travis.yml I get the above error. Linked npm packages and installed remote packages should behave the same, in my opinion. I think we do sth. strange in the styleguide plugin.

[appinteractive]

Sure I will take a closer look definitely! The reason for the code is the live reload as I don’t want to rebuild everything every time. This can’t be solved by npm link as it’s a webpack config. I think a solution or compromise is somewhere in between.

[appinteractive]

So I found several issues @roschaefer:

1. npm link does NOT work at all, it makes some magic which soes not work correctly with libraries as it seams. I get the export 'default' (imported as 'Styleguide') issue ONLY when using yarn link. Somehow you can`t use precompiled scripts with it for what ever reason. A reason to not use it. But it might be stable for the dev setup which would be better then relying on the parent folder, I will check.
   https://github.com/Human-Connection/Nitro-Web/pull/208/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2
2. there are issues with failing cypress tests which seams to come from ether the old electron browser in headless mode or/and an issue with nuxt.js (@nuxt/babel-preset-app ignores .browserlistrc nuxt/nuxt#5026, Unexpected token '...'. Expected a property name in Safari nuxt/nuxt#5065, Using spread operator with arrays requires polyfills babel/babel#4922) No soluton currently available, next nuxt.js version will get a fix which might fix it for us.

[roschaefer]

@appinteractive yarn link on my machine creates a symlink. That's it.

[roschaefer]

@appinteractive did some research. The error shows up if you symlink manually, e.g. ln -s ... but it does not show up if you just copy files cp ... 😲 this should definitely not happen.

[roschaefer]

@appinteractive nuxt/nuxt#3253 (comment)

[appinteractive]

Ahh that’s interesting, thanks. I will encapsulate the hot reload part who’s week.