Fix/Support npm file references

[HyperBrain]

What did you implement:

Closes #263

How did you implement it:

Local module reference paths are now rebased to the package directories where the package.json is, that is used to install the dependency. This will let npm find the local module (what was the actual error) and package it correctly.

How can we verify it:

Use a file: reference in the dependencies and do a serverless package. The command should succeed and the local module should exist in the packaged ZIP's node_modules folder.

Todos:

• Write tests
• Write documentation
• Fix linting errors
• Make sure code coverage hasn't dropped
• Provide verification config / commands / resources
• Enable "Allow edits from maintainers" for this PR
• Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

[franciscocpg]

@HyperBrain
The fix-npm-file-references branch is not working if a package-lock.json already exists in the root folder of the project (probably because of npm/npm#19183).

Unfortunately, I still had not seen this PR so I ended up implementing my own solution.

It's 100% working for my project.

Now that I realized this PR already existed we could put efforts in this PR or maybe follow with my approach.

What do you think @HyperBrain?

[HyperBrain]

@franciscocpg Yes, see #263 where I documented the npm bug. I created that when I ran into the issue, but until now nothing happened there.

The reason why it does not work is exactly the wrong handling of npm, because it does not check changes in the package.json files but only uses package-lock in the case of file: references.
The problem is, that we have to rebase the refs to point to the correct directories again when the package.json is assembled from the dependencies webpack encountered.

[HyperBrain]

Just had a look at your solution.

Although it works, the only publicly documented and accessible file that npm gives us is the package.json.
We should treat the package-lock file as internal to npm, and should not change anything in there. E.g. I noticed changes in the layout (addition/removal of fields) of the lock file between different npm versions (5.x). Especially, if the bug is fixed in npm, our lockfile modification might lead to other incompatibilities.
Modifying the lock file itself imposes the risk, that the plugin might break on any npm updates.

[franciscocpg]

I see.

But I think that while npm/npm#19183 isn't fixed the only way to get this relative modules working is modifying package-lock.json (regardless of which approach we use).

As a workaround, maybe we should use the approach you are using in this PR and to do the same file: replace you did in package.json in package-lock.json either. We could leave a comment in the code pointing to npm/npm#19183 and explaining this decision. When npm/npm#19183 gets fixed we could remove this replace.

What do you think @HyperBrain ?

[HyperBrain]

Sounds like a good idea. Just let's additionally add the rebased paths to the lock file. Then it should be quite stable.
If you want, you can do a PR onto the "fix-npm-file-references" branch and add the package-lock change there.

[franciscocpg]

ok, leave it with me.

[HyperBrain]

@franciscocpg I'm doing some tests with a bigger project right now. If it correctly packages the local modules now, I will merge it. 4.2.0 will be released soon (in case we do not have any other issues fixed). Thanks for the help on this and the great speedup of the feature 😃

[HyperBrain]

Did some tests. On Linux (Ubuntu) everything worked as expected, on Windows I got some link EPERM errors when npm tries to create its symlink, but that is expected, because "npm install <file ref>" and "npm link <fileref>" lead to the same errors locally. It seems my filesystem rights are screwed somewhere.
So, from my side everything is ok.
Anyone did tests on OSX?

[franciscocpg]

I have a VMWare OSX box in my personal laptop.

I can try it later.

[chronotc]

Hi,

Just tested this on OSX and it appears to be picking up the file referenced dependencies correctly.
npm 5.4.2, webpack 3.10, serverless 1.23

[franciscocpg]

Thank you @chronotc.

Did you test with and without package-lock.json in the project root folder?

[chronotc]

@franciscocpg
Tested this with package-lock.json present using SLS_DEBUG=* sls deploy -s dev and can confirm that its using Serverless: Package lock found - Using locked versions and packing external file referenced dependencies correctly.

Tried the same thing with package-lock.json removed and using the same process. Seems to be picking up the file referenced dependencies correctly.

Thanks for the help guys 👍 as the lack fo file referenced dependency support prevented us from adopting serverless-webpack.

[franciscocpg]

@HyperBrain
IMO it's ready to be merged.

[HyperBrain]

Will do soon 😀 Francisco Guimarães <notifications@github.com> schrieb am Mi., 20. Dez. 2017, 11:45 nachm.:

…

@HyperBrain <https://github.com/hyperbrain> IMO it's ready to be merged. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#281 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFRM3l21CTl4znV0B_0ZLIgpML_LUiRyks5tCY4IgaJpZM4Qh7rz> .

[HyperBrain]

Will release 4.2.0 with this fix now, as other features/fixes are not yet ready.