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

Search name #35

Closed
wants to merge 3 commits into from
Closed

Search name #35

wants to merge 3 commits into from

Conversation

aminya
Copy link

@aminya aminya commented Sep 2, 2020

Try to fix #22

This uses old JavaScript syntax. I am not sure why but it seems you want to support very old JavaScript.

@aminya
Copy link
Author

aminya commented Sep 10, 2020

@mafintosh I am blocked on this. If you can solve the issue it would be nice.

@mafintosh
Copy link
Collaborator

Unsure what is blocked and what the feature does.

@aminya
Copy link
Author

aminya commented Sep 10, 2020

The goal is to provide a name as the second argument, and the script searches for that package if it was not in the first argument:

var binding = require('node-gyp-build')(__dirname, "my-native-module")

@vweevers
Copy link
Member

@aminya Can you provide an example? What is the expected directory structure?

Lack of namespacing (other than by node_modules/xyz) is a known problem (#27) but I'm not sure how this PR solves it or that it covers all cases.

@aminya
Copy link
Author

aminya commented Oct 20, 2020

@aminya Can you provide an example? What is the expected directory structure?

Lack of namespacing (other than by node_modules/xyz) is a known problem (#27) but I'm not sure how this PR solves it or that it covers all cases.

You can't put index.js anywhere else other than the root of the repository. Builders change the __dirname of a file when they are built. For example, the src code is written to dist.

Here is an example. Try to move index.js to src folder, set the main to dist, and build the code. Everything breaks.
https://github.com/atom-ide-community/fuzzaldrin-plus-fast

@mafintosh
Copy link
Collaborator

I don't think that belongs here. The node-gyp-build interface tells you to pass in the directory where the prebuilds folder is, thats the contract.

@vweevers
Copy link
Member

vweevers commented Oct 25, 2020

Here is an example. Try to move index.js to src folder, set the main to dist, and build the code. Everything breaks.
https://github.com/atom-ide-community/fuzzaldrin-plus-fast

Simple solution: create an /index.js entry point that does module.exports = require('./dist/')(__dirname). Or in dist/index.js do path.dirname(__dirname).

I'm closing this PR seeing as it can be solved cleanly on your end, without increasing complexity here or introducing new assumptions about the directory structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The loading algorithm doesn't work for Webpack-bundled applications when dependencies are bundled into them.
3 participants