fix: change bundling to support linked libraries

[fahrradflucht]

2d9c337 introduced a check that throws if a file is neither in the cache nor contains 'node_modules' in its filename. This results in the bundler throwing on linked dependencies.

This PR removes the check on the filename. If a file can't be resolved patternplate-server won't exit anymore but should still log out errors.

[marionebl]

Awesome!

Could you add a test case at test/test-index.js for this? Thank you @fahrradflucht!

[marionebl]

Thinking about this a bit more:

What's the behaviour for available relative require ids after your change? The intended behaviour is to throw an Error with a descriptive error message.

[fahrradflucht]

I'm not sure if I understand that. Do you mean something like importing a present ./foo in index.jsx ? If so, this didn't throw this particular error message in the first place as far as my testing goes (I used patternplate-getting-started).

[marionebl]

I added a demo branch to patternplate-getting-started to demonstrate the intended behaviour:
https://github.com/sinnerschrader/patternplate-getting-started/blob/demo/throwing-relative-require/patterns/atoms/button/index.js#L1

According to my test the require statements in index.js and index.jsx both produce exception in the lieu of:

Cannot find module './test' from '~/patternplate-getting-started'

Does your change affect this behaviour?

Currently there are some kinks to sort out about error propagation, so looking at the terminal session patternplate runs in should be the current source of truth for exceptions.

[fahrradflucht]

Okay now I get it. I thought you meant Could not resolve ${file} as ${id} from ${context.file.path} should fire.

This doesn't get changed by this PR. The expected errors still fire without stopping the server.

[marionebl]

Nice! With a matching test case I'd merge this happily.

[fahrradflucht]

@KnisterPeter I wrote two tests like you suggested (one linked and one unlinked). This isn't Windows proof and the linter fails.

However I pushed it because I had the following problem:

The linked test that should fail without my change doesn't. This seems to be that case because the symlink isn't resolved at this point which causes the problem this PR tries to solve. But this is the case if you test this in a patternplate app. Do you have any ideas how I could fake this?

[KnisterPeter]

@fahrradflucht There are two problems right now. First of all you have to fix the linter warnings. Second it seems that your createLinkedFakeModule function fails.

[fahrradflucht]

@KnisterPeter Yes there is a problem permission problem with creating symlinks in the node_modules folder on travis and appveyor and disabling cashing doesn't fix it.

However as I said maybe it's pointless to fix this because the whole approach isn't working. Locally on my machine this test passes before and after my fix which means the test is wrong.

This is because if there is just this plugin the resolve algorithm seems to just resolve to path which makes this whole change unnecessary. This is however not the case in a full patternplate app (and isn't Node.js's behaviour either). There something looks up fs.realpath. But I can't figure out what and how to mimic that in the tests.

[KnisterPeter]

Remove caching for tests is not a solution anyway. Your test setup should create the state you need to run your tests and afterwards restore the previous state.
It would be even better to do all that in a different directory (fake) to do not interfer with other tests while they might execute (in case of a parallel test runner).