Skip to content

Conversation

@adroitwhiz
Copy link
Contributor

Resolves

Resolves #4965

Proposed Changes

This removes resolve {symlinks: false} from the Webpack configuration.

Reason for Changes

This allows Webpack to pick up on changes to npm linked Scratch modules and live-recompile + reload them. While the documentation says the "resolve symlinks" option "may cause module resolution to fail when using tools that symlink packages (like npm link)", it is unclear whether enabling or disabling it causes the failure, and in my case it only worked properly when set to true.

My development environment is Linux-based, so I'm not sure how this affects other OSes.

@thisandagain
Copy link
Contributor

/cc @paulkaplan @rschamp @benjiwheeler

@rschamp
Copy link
Contributor

rschamp commented Jul 8, 2019

When #2012 went in, and we didn't have this option, we couldn't build at all while linked, and got errors like:

    Module build failed: ReferenceError: Unknown plugin "react-intl" specified in "base" at 3, attempted to resolve relative to "/Users/rschamp/Documents/Scratch/scratch-audio/src"
        at /Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:180:17
        at Array.map (native)
        at Function.normalisePlugins (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:158:20)
        at OptionManager.mergeOptions (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:234:36)
        at OptionManager.init (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/file/options/option-manager.js:368:12)
        at File.initOptions (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/file/index.js:212:65)
        at new File (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/file/index.js:135:24)
        at Pipeline.transform (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-core/lib/transformation/pipeline.js:46:16)
        at transpile (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-loader/lib/index.js:50:20)
        at Object.module.exports (/Users/rschamp/Documents/Scratch/scratch-gui/node_modules/babel-loader/lib/index.js:173:20)
     @ ./src/containers/gui.jsx 11:20-44
     @ ./src/playground/index.jsx
     @ multi (webpack)-dev-server/client?http://0.0.0.0:8601 ./src/playground/index.jsx

Since that's much more disruptive than live rebuilding not working, I'm currently not inclined to merge this. First I'm going to do some testing with this branch to see if I can reproduce that issue though.

@rschamp
Copy link
Contributor

rschamp commented Jul 8, 2019

I don't see the same errors as I had been seeing, so I think this may be merge-able. I will investigate a bit more first.

@rschamp
Copy link
Contributor

rschamp commented Jul 9, 2019

I confirmed that one of the members of our team would see errors if they tried to build with a package linked and this option removed, so unfortunately I don't think we should merge this.

I wanted to suggest that you use a CLI option, but it looks like they provide a way to set every resolve option from the command line except symlinks :(

Thanks for your patch though! We should revisit this when it's more certain nobody is using npm < 6.

@rschamp rschamp closed this Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recompiling npm linked Scratch modules doesn't trigger a Webpack rebuild

3 participants