Skip to content
This repository has been archived by the owner on Jan 6, 2022. It is now read-only.

Prevent "webpacking" of native dependencies #508

Merged
merged 1 commit into from Feb 7, 2018
Merged

Prevent "webpacking" of native dependencies #508

merged 1 commit into from Feb 7, 2018

Conversation

martinheidegger
Copy link
Collaborator

During debugging some connection issue on the react branch I found that utp-native couldn't be loaded and as a result utp wasn't available.

utp is loaded through node-gyp-build whichs load-method has a wildcard-require, resulting in following warning:

WARNING in ./node_modules/node-gyp-build/index.js
13:9-32 Critical dependency: the request of a dependency is an expression
    at CommonJsRequireContextDependency.getWarnings (/Users/m/Documents/dat-desktop/node_modules/webpack/lib/dependencies/ContextDependency.js:39:18)
    at Compilation.reportDependencyErrorsAndWarnings (/Users/m/Documents/dat-desktop/node_modules/webpack/lib/Compilation.js:703:24)
    at Compilation.finish (/Users/m/Documents/dat-desktop/node_modules/webpack/lib/Compilation.js:561:9)
    at applyPluginsParallel.err (/Users/m/Documents/dat-desktop/node_modules/webpack/lib/Compiler.js:502:17)
    at /Users/m/Documents/dat-desktop/node_modules/tapable/lib/Tapable.js:289:11
    at _addModuleChain (/Users/m/Documents/dat-desktop/node_modules/webpack/lib/Compilation.js:507:11)
    at processModuleDependencies.err (/Users/m/Documents/dat-desktop/node_modules/webpack/lib/Compilation.js:477:14)
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)

This means that as-is webpack can not find the depending packages. This PR marks all node-modules as external, which puts it in a similar build structure as is currently used by the choo-builds and thus allows access to utp-native.

Copy link
Collaborator

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

aw yes, very good and great find!

@martinheidegger martinheidegger merged commit 659b8de into dat-ecosystem-archive:react Feb 7, 2018
@martinheidegger martinheidegger deleted the fix/react-native-deps branch February 7, 2018 11:07
@juliangruber
Copy link
Collaborator

@martinheidegger please wait for at least 48h so @aks- also has time to review

@martinheidegger
Copy link
Collaborator Author

Sorry for that faux-pass I hope it's okay this time? Should I revert the commit?

@juliangruber
Copy link
Collaborator

I think it's fine :) Just want to remind you of the culture we want to establish :)

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.

2 participants