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

Start module resolution from mainModulePath #30

Closed
RubenVerborgh opened this issue Jan 1, 2021 · 8 comments
Closed

Start module resolution from mainModulePath #30

RubenVerborgh opened this issue Jan 1, 2021 · 8 comments

Comments

@RubenVerborgh
Copy link
Contributor

RubenVerborgh commented Jan 1, 2021

I wonder if the code at https://github.com/LinkedSoftwareDependencies/Components.js/blob/v3.6.1/lib/factory/UnnamedComponentFactory.ts#L191-L194:

                    // Always require relative from main module, because Components.js will in most cases just be dependency.
                    object = require.main.require(requireName.charAt(0) === '.'
                      ? Path.join(process.cwd(), requireName)
                      : requireName);

should actually be:

                    object = require(requireName.charAt(0) === '.'
                      ? Path.join(process.cwd(), requireName)
                      : require.resolve(requireName, { paths: [ Util.getMainModulePath() ] }));

I currently have a folder structure as follows:

  • projects
    • solid-hue
      • node_modules
        • symlink to ../../community-solid-server
    • community-solid-server
      • node_modules
        • componentsjs

So when I invoke the server from community-solid-server with solid-hue as mainModulePath, the componentsjs function require will not find community-solid-server solid-hue because it is in a different tree.

Therefore, I think that module resolution should be relative to the mainModulePath.

@rubensworks
Copy link
Member

Since there's a symlink to community-solid-server, the standard require should be able to find it though.
External comunica modules are developed in the same way, so that should just work (unless I'm missing something else).

Does a manual require to CSS work when inside the solid-hue scope?

@RubenVerborgh
Copy link
Contributor Author

Since there's a symlink to community-solid-server, the standard require should be able to find it though.

I miswrote above; corrected now: solid-hue is the module that cannot be found. It'scommunity-solid-server that has no symlink to solid-hue, and module resolution starts in the node_modules folder of community-solid-server, never reaching solid-hue.

@rubensworks
Copy link
Member

Hmm, I have similar Comunica setups, so it should be possible to make this work though.

Can we put this on hold until the new Components.js release is out?
I changed quite a bit of things related to module resolution (also better tested), so this may very well already be fixed.

@RubenVerborgh
Copy link
Contributor Author

Hmm, I have similar Comunica setups

The key difference here is probably that I am starting the community-solid-server executable from within community-solid-server, so it does not have access to the solid-hue tree.

I expect that you are probably instantiating from the separate module directly.

Can we put this on hold until the new Components.js release is out?

That's fine with me, but I expect that a similar decision will need to be taken there. The fundamental question is:

  • Should module resolution start from
    • the path where the active componentsjs is located (require.main)?
      • projects/community-solid-server/node_modules in my case (can't find solid-hue)
    • the specified mainModulePath?
      • projects/solid-hue in my case (can find solid-hue and symlinked community-solid-server)

I suspect that, in your cases, both happen to have been the same path.

@rubensworks
Copy link
Member

You may be right, my cases are indeed always instantiating themselves.

Should module resolution start from
the specified mainModulePath?

This is probably what we want. But I'll have to check to make sure that we're not missing something, as to not break existing setups.

@RubenVerborgh
Copy link
Contributor Author

Confirmed failing with version 4.

To reproduce:

git clone [email protected]:RubenVerborgh/solid-hue.git
cd solid-hue
git checkout feat/componentsjs-4
npm ci
npm ln @solid/community-server // checked out to https://github.com/solid/community-server/pull/505
npx community-solid-server -c settings.json -m .

Results in

Error: Cannot find module '@solid/community-server'
Require stack:
- /Users/ruben/Documents/UGent/Solid/solid-community-server/bin/server.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:1029:15)
    at Function.Module._load (internal/modules/cjs/loader.js:898:27)
    at Module.require (internal/modules/cjs/loader.js:1089:19)
    at ConstructionStrategyCommonJs.createInstance (/Users/ruben/Documents/UGent/Solid/solid-community-server/node_modules/componentsjs/lib/construction/strategy/ConstructionStrategyCommonJs.js:26:36)
    at ConfigConstructor.createInstance (/Users/ruben/Documents/UGent/Solid/solid-community-server/node_modules/componentsjs/lib/construction/ConfigConstructor.js:75:42)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

componentsjs-error-state.json.log

@rubensworks
Copy link
Member

@RubenVerborgh Your suggested fix above seems to do the trick (and breaks nothing else).
I'll release a Components.js fix in a bit.

FYI, you'll also have to update the generator to 2.0.0 to make things work (needed for JSON-LD 1.0 -> 1.1).

@RubenVerborgh
Copy link
Contributor Author

Your suggested fix above seems to do the trick

🤘

(and breaks nothing else)

🤘🤘🤘🤘🤘

JSON-LD 1.0 -> 1.1

Ooh, exciting.

Thanks!

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

No branches or pull requests

2 participants