Skip to content

[#74] feat: add imported css to its importer #75

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 3 commits into from
Jan 23, 2023
Merged

[#74] feat: add imported css to its importer #75

merged 3 commits into from
Jan 23, 2023

Conversation

mseyfayi
Copy link
Contributor

No description provided.

Copy link
Owner

@marco-prontera marco-prontera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, thank you for your contribution! 👍

We need to make this behavior configurable since currently the library is used with the existing behavior.

What we can do is:

  • Make this behavior configurable (I know you think this is a bug, but we need to think of this as an improvement)

Someone could have the need to inject the CSS once and for all since this can impact performance in certain cases. The configuration flag can be named as 'relativeCSSInjection' a boolean when true the behavior you proposed will be enabled.

@marco-prontera marco-prontera linked an issue Jan 22, 2023 that may be closed by this pull request
@marco-prontera marco-prontera added the enhancement New feature or request label Jan 22, 2023
@marco-prontera marco-prontera changed the title fix: add imported css to its importer feat: add imported css to its importer Jan 22, 2023
@marco-prontera marco-prontera changed the base branch from main to develop January 22, 2023 14:25
@mseyfayi
Copy link
Contributor Author

First of all, thank you for your contribution! 👍

We need to make this behavior configurable since currently the library is used with the existing behavior.

What we can do is:

  • Make this behavior configurable (I know you think this is a bug, but we need to think of this as an improvement)

Someone could have the need to inject the CSS once and for all since this can impact performance in certain cases. The configuration flag can be named as 'relativeCSSInjection' a boolean when true the behavior you proposed will be enabled.

relativeCSSInjection has been added, and it will switch between adding all CSSs or just imported CSSs

Copy link
Owner

@marco-prontera marco-prontera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!
Give me the time to test locally and write documentation and I will notify you of the new release!

@marco-prontera marco-prontera changed the title feat: add imported css to its importer [#74] feat: add imported css to its importer Jan 23, 2023
@marco-prontera marco-prontera merged commit 3a2d006 into marco-prontera:develop Jan 23, 2023
@mseyfayi mseyfayi deleted the feature/74 branch January 23, 2023 19:37
@marco-prontera
Copy link
Owner

@mseyfayi Does this feature work with clean usage (without vue or anything) or am I doing something wrong? If you have time can you give me an example? A test repository because with my tests I can't get the desired result, maybe I'm doing something wrong.

Thanks in advance!

@mseyfayi
Copy link
Contributor Author

I apologize for not getting back to you sooner.
I only test it in React, and you're right it should be tested in Vue and Svelte, and other frameworks. But since we are working with the Rollup, not the framework plugins, I don't think there would be any problem.

But still, I'm not sure!

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

Successfully merging this pull request may close these issues.

Add imported css to its importer
2 participants