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

Build system: do not use require.resolve for resolving library file paths #8592

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

dgirardi
Copy link
Collaborator

Type of change

  • Build related changes

Description of change

Use path.resolve instead of require.resolve when resolving library file paths. require.resolve seems to behave unexpectedly in certain situations (#8588)

Other information

Closes #8588

@dgirardi
Copy link
Collaborator Author

@karimMourra could you confirm that the use of require.resolve was incidental - or did you want that to work with node_modules somehow?

@ChrisHuie ChrisHuie self-requested a review June 21, 2022 21:01
@ChrisHuie ChrisHuie self-assigned this Jun 21, 2022
@karimMourra
Copy link
Collaborator

@dgirardi the decision was based on the steps for modules:
var modulePath = require.resolve(module, {paths: [MODULE_PATH, LIBRARY_PATH]}); in getModules. I notice that the line is wrapped in a try-catch block, perhaps because of the node-module issues you mention.
I am OK with using path.resolve. Thank you for fixing!

@ChrisHuie ChrisHuie merged commit 60e2466 into prebid:master Jun 23, 2022
renebaudisch pushed a commit to renebaudisch/Prebid.js that referenced this pull request Jun 28, 2022
jlaso pushed a commit to AuDigent/Prebid.js that referenced this pull request Jul 1, 2022
bwhisp pushed a commit to bwhisp/Prebid.js that referenced this pull request Jul 13, 2022
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.

gulp build/test failing on Mac OS
3 participants