Plugin not working when moment-timezone is not in 'node_modules' directory

[diniciacci]

Currently the filter does not work if moment-timezone is located in a different directory than 'node_modules'.

This is due to line 199 where 'node_modules' is hard coded in the path to match the context.

Having modules located in other directories is supported by webpack as version 5 and also version 4.

IMHO a fix could be to just match '/moment-timezone' in the context.

On the contrary a more extended fix could involve matching all the values of resolve.modules array + 'moment-timezone'.

I can provide a PR if you want.

[gilmoreorless]

Good catch! I've used resolve.modules in some projects before, but not with moment-timezone, so I hadn't come across this problem.

For your use case, when you have moment-timezone in a different directory, does it still have the same file/directory structure as it would in node_modules? I just want to make sure that require('moment-timezone/data/packed/latest.json') would still work as expected.

[diniciacci]

Thanks ;)

My use case comes from simply porting a legacy code to webpack and, as not so uncommon, the libraries for the client are located in a "vendor" directory. In particular this is a dojo toolkit project, that had its own (very good) build system that stored libs in arbitrary (configurable) paths.

To sum it up, yes in my case moment-timezone was just located in "vendor" instead of "node_modules". In general for dojo that might not be the case.

Notice that from my few tests for my use case, there is no way to make the plugin work without editing its code. This is because to me it looks like webpack will always resolve the context to the actual absolute path (e.g. even if you create symlinks or try to circumvent the loading location with aliases).

Imho also fixing 'moment-timezone/data/packed/latest.json' scares me a bit and maybe should be a configurable value? timezoneDataFilename? Nonetheless in my usecase the data filename was not a problem.

Let me know if I can be of any help, I'm currently hunting another one around here ;)

[gilmoreorless]

That's good context, thanks.

Imho also fixing 'moment-timezone/data/packed/latest.json' scares me a bit and maybe should be a configurable value? timezoneDataFilename?

Yep I have wondered in the past about that hard-coded reference, but I left it alone until an option for it was actually needed. It looks like you've found that use case.

I'm a bit pressed for time at the moment, so if you're happy to put up a PR about this, it would be super-helpful.

[diniciacci]

Hey,

the PR is ready, but I'm stuck on the test. I would like to provide something more than a unit test, because the modification, albeit small, has quite some use consequences. I.e. I need a brilliant idea on how to test it.

[gilmoreorless]

Ah yes, that's an interesting question. Some of the tests already use memory-fs for checking the webpack-compiled output. Perhaps you could create a new MemoryFS as the input filesystem and write whatever temporary files you need to it? (I haven't actually tried this myself.)

[diniciacci]

Thanks, this is exactly the kind of suggestion I was looking for. I will try this way and see if I can get it running.

[diniciacci]

I did the pull request as the code was getting stale. The reason for the wait is that there is one missing test that I really wanted for the code.

I left the missing test string commented in case someone gets a good idea on how to code it properly. My only idea would have meant heavily touching the current test harnesses and looked like an overkill.

I didn't continue on the memory-fs path because it is deprecated and getting even more dependent on it sounded bad to me. At the same time moving everything to memfs looked a bit out of scope of the fix.

[gilmoreorless]

I took a quick look at that commented-out test. You're right, it would be best to switch to memfs (and linkfs) to implement it properly. I've created #36 to tackle that separately.

I've released your PR as version 1.4.0 so that it's not held up by the test fixes.

[diniciacci]

Thanks. I really agree with you. I totally missed linkfs and it is indeed very interesting. I'll have a look too.