-
Notifications
You must be signed in to change notification settings - Fork 4
Reimplemented the plugin for bless-4.0.0 #2
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
Conversation
Fly-by: - Fixed readme - Moved test folder to demo folder (it doesn't contain any actual tests)
This looks fantastic! The only missing link here are unit tests, your demo is serving as a manual test, though automated unit/integration tests would really help for maintainability. |
We do have an active issue to reimplement the imports in the parser, and I like what you're doing here to generate them. Maybe not right now, but I would like to see this logic moved over and we can just use a bless option to trigger imports. |
if (index === blessed.data.length - 1) { | ||
name = fileName; | ||
const imports = generateImports(fileName, blessed.data.length - 1) ; | ||
chunk = `${imports}\n${chunk}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this affect sourcemaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right, completely missed that. I'll get it fixed.
I agree that it might be better to move the logic over to the main project. I didn't realize that imports where completely missing still from the main project, they were present in 3.x and I just assumed that only this API method didn't generate them. I'll try to see if I can create a PR for this for the main project this weekend. |
It was on our todo list, we just never had the time when working on the |
Would be cool if we could add the functionality from #4 as well. I can take a shot at this if you would like some help. |
Is this getting merged ever? |
@mtscout6 do you plan to update bless version from 3.0.3 to 4.0.4? :) |
There are other members of the org that can do this. I don't personally have the time right now. @BlessCSS/owners @BlessCSS/contributors |
I also adopted the code styling from the main project and moved everything to the BlessCSS repo.
@BlessCSS/owners - Could you take a look at this and provide feedback where needed?