search

jamesandersen / string-replace-webpack-plugin

Replace string tokens in a bundle.
93 stars 27 forks source link

Incompatible with recordsPath #25

Open rowanoulton opened 7 years ago

rowanoulton commented 7 years ago

Hey, firstly: thank you for the incredibly useful plugin.

I'd like to use it in conjunction with recordsPath and CommonsChunkPlugin in order to achieve long-term caching but am seeing the records file filled with duplicates after each run. I think this is due to the random query ID added here. This creates many references to the same file like so:

/* in records.json */
/* ... */
"node_modules/string-replace-webpack-plugin/loader.js?id=<hash>!path/to/file.js": 1,
/* then, later: */
"node_modules/string-replace-webpack-plugin/loader.js?id=<another-hash>!path/to/file.js": 2,

(a new copy is added to the records file for each subsequent webpack run)

My assumption is that this ID is necessary in order for the plugin to be used multiple times in the same config. In that case, I wonder if the best solution here is to allow consumers of the plugin to pass in an ID directly, eg:

loader: StringReplacePlugin.replace({
        id: 'some-identifier',
    replacements: [
        {
            pattern: /thing/,
            replacement: /* ... some function .. */
        }
    ]
})

I'd be happy to submit a PR to do the above, but wanted to check with you to see if it sounds reasonable first. Let me know what you think ✌

jamesandersen commented 7 years ago

@rowanoulton glad you've found the plugin useful! I just wanted to reply back and let you know I've been in a bit of a busy stretch recently and not able to give this plugin much attention. A PR is welcome but it might take me a little while to review and merge it (wasn't actually aware of recordsPath until your note here so it'll be a good opportunity to understand it better).

As this issue sounds a bit tricky, a unit test or two in the PR sounds like it would be advisable.