Skip to content

Update ResolverFactory.js#286

Closed
dlredden wants to merge 1 commit intowebpack:masterfrom
dlredden:patch-1
Closed

Update ResolverFactory.js#286
dlredden wants to merge 1 commit intowebpack:masterfrom
dlredden:patch-1

Conversation

@dlredden
Copy link
Copy Markdown

Issue #263 is preventing my company from upgrading to Webpack v5. This was the proposed fix that worked well for us so I created a PR.

Issue webpack#263 is preventing my company from upgrading to Webpack v5. This was the proposed fix that worked well for us so I created a PR.
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 21, 2021

Codecov Report

Merging #286 (2e296b8) into master (ff16fc2) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #286   +/-   ##
=======================================
  Coverage   94.93%   94.93%           
=======================================
  Files          39       39           
  Lines        1579     1579           
=======================================
  Hits         1499     1499           
  Misses         80       80           
Impacted Files Coverage Δ
lib/ResolverFactory.js 97.09% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff16fc2...2e296b8. Read the comment docs.

@alexander-akait
Copy link
Copy Markdown
Member

Please provide example how we can reproduce the problem

@ferdinando-ferreira
Copy link
Copy Markdown

Please provide example how we can reproduce the problem

@alexander-akait : https://github.com/ferdinando-ferreira/enhanced-resolve-pnpapi-bug

git clone https://github.com/ferdinando-ferreira/enhanced-resolve-pnpapi-bug.git enhanced-resolve-pnpapi-bug
cd enhanced-resolve-pnpapi-bug
npm install
npm run build

@alexander-akait
Copy link
Copy Markdown
Member

@ferdinando-ferreira Why do you bundle webpack? You should ignore this module (resolve.fallback.pnpapi: false) when you bundle webpack for browser, because browsers doesn't support pnp

@ferdinando-ferreira
Copy link
Copy Markdown

Why do you bundle webpack?

@alexander-akait : To provide the minimal usecase requested.

Regardless of usefulness it provides the exact error case reported:

require("pnpapi")

when it should be

require("./PnpPlugin").PnpApiImpl

because there is no pnpapi package.

Would you prefer a "realistic" use case that would hit the same error?

@alexander-akait
Copy link
Copy Markdown
Member

alexander-akait commented Mar 25, 2021

Please read https://yarnpkg.com/advanced/pnpapi, require("pnpapi") !== require("./PnpPlugin").PnpApiImpl

@ferdinando-ferreira
Copy link
Copy Markdown

ferdinando-ferreira commented Mar 25, 2021

Please read https://yarnpkg.com/advanced/pnpapi, require("pnpapi") !== require("./PnpPlugin").PnpApiImpl

You are correct! The root cause of the error must be then the original issue submitter accidentally bundling something with a require to webpack (or other package depending on enhanced-resolve) with target: web.

@dlredden, @dstampher: any chance that's the case?

@alexander-akait
Copy link
Copy Markdown
Member

Yep, in this case resolve.fallback.pnpapi: false should solve the problem

@sokra
Copy link
Copy Markdown
Member

sokra commented Apr 19, 2021

If you want to use PnP with the compiled bundle, it's not resolve.fallback but externals instead:

externals: {
  pnpapi: true
}

sokra added a commit to webpack/webpack that referenced this pull request Apr 19, 2021
@dlredden dlredden deleted the patch-1 branch June 15, 2021 15:38
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.

5 participants