Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DLL support to CRA #2710

Closed
wants to merge 6 commits into from
Closed

Add DLL support to CRA #2710

wants to merge 6 commits into from

Conversation

asfktz
Copy link

@asfktz asfktz commented Jul 1, 2017

@gaearon
Copy link
Contributor

gaearon commented Jul 1, 2017

This is very cool stuff. However, I think we can't take it until it gets more real world usage. Do you mind working with webpack folks to find good first projects to try it? Perhaps in some webpack issue where people complain about build times. Once bugs are squashed and limitations are known we can come back to this.

@viankakrisna
Copy link
Contributor

Nice! I agree with @gaearon, as this counts as third party deps, it needs to be battle tested first.

We have #1651 though :p

@asfktz
Copy link
Author

asfktz commented Jul 2, 2017

Hi @gaearon!
That is very honest and responsible answer, I respect that (:
I would love to work with Webpack's folks to battle test the project first.

Also, your tweet brought a lot of traffic to the Repo (thanks by the way!), and a couple of issues and PRs started to pile up.

Most of them related to using DLL in production which is an interesting use case, I only thought about it as a development tool till now, but it makes sense.

@gaearon
Copy link
Contributor

gaearon commented Jul 2, 2017

Another thing I don't understand is invalidation. How do you handle that? For example if I want to edit something in node_modules because I'm debugging some issue. Does it trigger a rebuild?

@asfktz
Copy link
Author

asfktz commented Jul 2, 2017

Great question.

I use find-cache-dir to create the cache dir inside node_moduels

image
Every time you npm install or remove a module, NPM deletes the .cache directory.
So I check if it exists, if not, it will trigger a build before the next compile.

I also store the plugins settings as lastSettings.json and compare it to the current settings.

@gaearon
Copy link
Contributor

gaearon commented Jul 2, 2017

Right, but I mean the case where somebody might put console.logs into library code in node_modules to temporarily debug things.

@viankakrisna
Copy link
Contributor

I've googled a bit maybe we can use this node equivalent bash command

tar -cf - node_modules | md5

To hash the contents of node_modules source

Another thing is that the DLL compiler should run in watch mode to catch the changed files and rebuild accordingly

@viankakrisna
Copy link
Contributor

(sorry if my comments are distracting, but I really excited to see how this feature landed :D)

@Timer
Copy link
Contributor

Timer commented Jul 2, 2017

Does Yarn delete the .cache directory? I think we've seen instances where it doesn't, just something to be mindful of -- but yes node modules may be edited manually.

@asfktz
Copy link
Author

asfktz commented Jul 2, 2017

(sorry if my comments are distracting, but I really excited to see how this feature landed :D)

@viankakrisna, not at all! you are most welcome.

Right, but I mean the case where somebody might put console.logs into library code in node_modules to temporarily debug things.

@gaearon in that case, yes, it will not invalidate.
That's why I decided not to add react & react-dom in the DLL by default.

Another thing is that the DLL compiler should run in watch mode to catch the changed files and rebuild accordingly

I agree that adding 'watch' functionally is a feature worth adding, but I think it should not be enabled by default. I fear that watching the entire node_moduels can have an impact on performance.

tar -cf - node_modules | md5 looks interesting. I will try it.
Otherwise, It can use something like watchman.

Does Yarn delete the .cache directory? I think we've seen instances where it doesn't, just something to be mindful of

@Timer, interesting, I tested it with both Yarn & NPM, can you remember a case where it doesn't?

@viankakrisna
Copy link
Contributor

@asfktz yeah, a node version of tar -cf - node_modules | md5 could be used in the initial phase to check whether or not the DLL needs to be rebuilt (it's reasonably fast). Then, watchman / webpack's built in watcher can takeover the cache invalidation.

I've been checking out npm packages for hashing a directory, but haven't tested the performance yet. https://github.com/mcavage/node-dirsum and https://github.com/marc136/node-folder-hash is some of it.

@asfktz
Copy link
Author

asfktz commented Jul 2, 2017

Hmm.. interesting!
Also, maybe we can watch only the modules specified in the DLL entries.

@viankakrisna
Copy link
Contributor

Hmm, it wouldn't work if we are editing a transitive dependency. But interesting to have it as an option :)

As for the case with .cache not deleted during yarn install, we can hash the package.json, yarn.lock and package-lock.json https://github.com/viankakrisna/create-react-app/blob/6270d63b20f177cff9b78eb83ff1bc1cd363c313/packages/react-dev-utils/webpackAutoDllCompiler.js#L114-L136

@asfktz
Copy link
Author

asfktz commented Jul 4, 2017

@viankakrisna would you like to join the to the discussion on asfktz/autodll-webpack-plugin#7?
It will be really nice to have you there (:

@miraage
Copy link

miraage commented Aug 8, 2017

Note: plugins from webpack will not apply to the AutoDllPlugin unless you provide them explicitly. For example:

// webpack.config.dev.js
module.exports = {
  plugins: [
    new webpack.NamedModulesPlugin(),
    new AutoDllPlugin({
      plugins: [
        new webpack.NamedModulesPlugin(), // won't apply unless you put this line
      ],  
    }).    
  ],
};

@@ -42,6 +42,8 @@
"extract-text-webpack-plugin": "2.1.2",
"file-loader": "0.11.2",
"fs-extra": "3.0.1",
"html-webpack-plugin": "2.28.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already has html-webpack-plugin 2.29.0. Can remove this

@@ -42,6 +42,8 @@
"extract-text-webpack-plugin": "2.1.2",
"file-loader": "0.11.2",
"fs-extra": "3.0.1",
"html-webpack-plugin": "2.28.0",
"jest": "20.0.3",
Copy link
Contributor

@andykenward andykenward Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"jest": "20.0.4" is already in packages.json. Should remove this line "jest": "20.0.3"

@andriijas
Copy link
Contributor

andriijas commented Apr 4, 2018

Webpack 4 has better capabilities than dll. It will be in cra soon.

@andriijas andriijas closed this Apr 4, 2018
@ghost
Copy link

ghost commented May 6, 2018

@andriijas What's the eta of webpack 4 to be in cra?

@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants