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

Improvements to localModules #169

Open
mariusGundersen opened this issue Nov 28, 2023 · 2 comments
Open

Improvements to localModules #169

mariusGundersen opened this issue Nov 28, 2023 · 2 comments

Comments

@mariusGundersen
Copy link
Contributor

Background: I'm working on a VSCode extension for Espruino.

The localModules file assumes that the source code is in the process.cwd(), which isn't always true. For my scenario the cwd is completely different from the file location, since the vscode window runs somewhere else entirely. It would be nice to be able to set the value of cwd somehow in the module.

There is also a problem with the loadEspModules function, which uses (essentially) fs.readFileSync(path.join('modules', moduleName)), which ends up with an implicit process.cwd().

I also don't think it behaves like require() should do, since it always loads it relative to the current process, not the file that requires it. For example, given that main.js: requires('./folder/file.js') and folder/file.js: requires('./anotherFile.js), then it should find folder/anotherFile.js, but since it doesn't use the current location of the file that required it, it ends up looking in the root folder. For this to work we need to know the path of the current module when it looks for the next module. It would then have to rewrite the code so that the url is absolute, for running on the device. This is a bit more complex to solve, and would have to involve the modules.js file too. The above changes can be done in localModules.js only.

@gfwilliams
Copy link
Member

Ok, thanks! Yes, that sounds like it's definitely something that needs changing - looking relative to the current file's directory seems the way to go.

While it might be nice to send the current file path in data for Espruino.addProcessor("getModule", function (data, callback) {, that itself is a bit tricky since when running in the browser we don't know the file path.

I guess we could do that and then at the very least we can pass the directory through when requiring one module from another module?

Up to you, but yes, as long as it doesn't break the IDE I'm all for getting this working nicely

@mariusGundersen
Copy link
Contributor Author

"getModule" is initially called from "transformForEspruino" which only had the code, not any other parameters, so it wouldn't be possible without breaking changes to pass in the cwd there. Instead there could be a Espruino.Plugins.LocalModules.SetCwd(cwd) method for overriding the process.cwd() default. This would have to be called before "transformForEspruino".

To make relative paths work there needs to be a lot of changes in Espruino.Core.Modules, which is why I started refactoring it. I'm not sure it's worth it, it seems to me like a better idea is to put all modules in the modules folder (and maybe have a config for it being called espruino_modules, as mentioned in the code comments).

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

No branches or pull requests

2 participants