Skip to content

Proposal: allow opening files from the current project globally #6483

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

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

zth
Copy link
Collaborator

@zth zth commented Nov 14, 2023

This is for discussion/a proposal.

We have a situation right now where you can use -open in bsc-flags to open modules globally. This is quite useful in many cases. However, you're only able to open modules from external dependencies, not modules from your current project, as that would create a circular dependency as it'd also be opened in itself. This PR changes that so that the globally opened module won't be opened in itself.

I'm not sure if this is something we want to allow, nor what the consequences would be. But, it would be useful in many scenarios given that you could put together your own globally available little "stdlib". The usefulness might be a bit negated by the fact that it'd be quite restricted - you couldn't reference other files in your project as those would get the open injected, and by that create a circular dep. It might also cause excessive compilation times when you change your globally opened file, as all project files will now depend on that.

So, unclear if the upsides beat the downsides.

Comments and discussion welcome!

@fhammerschmidt
Copy link
Member

Just to clarify: You can use open for your own namespaced packages in a npm workspace repo. They don't need to be externally installed deps.

@DZakh
Copy link
Member

DZakh commented Nov 15, 2023

Yeah, this is how we do it

@cknitt
Copy link
Member

cknitt commented Nov 22, 2023

This would certainly be practical as not everyone would want to have an npm/yarn workspace setup just to be able to globally open a module from the current project.

Will test it when I have some time.

@zth
Copy link
Collaborator Author

zth commented Dec 11, 2023

I'm thinking this doesn't hurt to merge, even if there are other solutions and this isn't optimal, it still feels like it should be allowed.

What do you think @DZakh @fhammerschmidt @cknitt ?

@cknitt
Copy link
Member

cknitt commented Dec 11, 2023

I like it! Sorry that I haven't actually tested it yet, I'll try to have a look tomorrow.

@zth zth requested a review from cristianoc December 11, 2023 19:45
@DZakh
Copy link
Member

DZakh commented Dec 11, 2023

Yes, I also think it's a good feature. And I can't remember the cases, but there were a few times when I needed it.

@DZakh
Copy link
Member

DZakh commented Dec 11, 2023

Should there be some work on rewatch side to add support for this?

@cknitt
Copy link
Member

cknitt commented Dec 12, 2023

Just tested, works fine! 👍

@zth
Copy link
Collaborator Author

zth commented Dec 13, 2023

Should there be some work on rewatch side to add support for this?

I don't think that's needed, this is in the compiler itself.

@zth zth merged commit e9fa3ed into master Dec 20, 2023
@zth zth deleted the allow-global-opens-from-same-project branch December 20, 2023 15:33
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.

4 participants