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

[Bug]: Front-end build touches *all* apps scss/css #38092

Closed
5 of 9 tasks
ChristophWurst opened this issue May 5, 2023 · 6 comments · Fixed by #38590
Closed
5 of 9 tasks

[Bug]: Front-end build touches *all* apps scss/css #38092

ChristophWurst opened this issue May 5, 2023 · 6 comments · Fixed by #38590
Labels
Milestone

Comments

@ChristophWurst
Copy link
Member

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • Nextcloud Server is running on 64bit capable CPU, PHP and OS.
  • I agree to follow Nextcloud's Code of Conduct.

Bug description

Running npm run build touches app scss and creates unexpected css files

I raised this earlier in #35622 (comment) already and a discussion spawned at nextcloud/standards#9 but until a decision has been made and docs still recommend people to clone into apps I think it's fair to say the server front-end should leave unrelated apps untouched.

Steps to reproduce

  1. Clone server
  2. Clone calendar into apps
  3. Compile the server front-end

Expected behavior

No changes in the calendar repo

Installation method

None

Nextcloud Server version

master

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

N/a

List of activated Apps

N/a

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

cc @skjnldsv @AndyScherzinger

@ChristophWurst ChristophWurst added bug 1. to develop Accepted and waiting to be taken care of technical debt labels May 5, 2023
@ChristophWurst ChristophWurst changed the title [Bug]: Front-end build touches *all* apps [Bug]: Front-end build touches *all* apps scss/css May 5, 2023
@ChristophWurst
Copy link
Member Author

Idea: read https://github.com/nextcloud/server/blob/master/core/shipped.json and only run the sass compiler for apps in there.

Added benefit: the local build with additional apps becomes faster.

@AndyScherzinger
Copy link
Member

Suggestion of @ChristophWurst would be fine with me, so looking for the feedback of @skjnldsv and depending on the efforts and availability I'd be okay with adding this for 27 during the stabilization phase. Not sure what you think @skjnldsv

@szaimen szaimen removed the bug label May 5, 2023
@skjnldsv
Copy link
Member

skjnldsv commented May 5, 2023

Sure :)
This is really a dev corner case.
It's the same for composer jobs like lint or php-cs

@ChristophWurst
Copy link
Member Author

The cs fixer is able to ignore anything ignored by git:

->ignoreVCSIgnored(true)

But yes, lint probably has the same issue. Albeit it doesn't modify additional apps.

@AndyScherzinger
Copy link
Member

So how much work do we expect this to be and who could take care of it?

@ChristophWurst
Copy link
Member Author

2h for an experienced frontender

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Jun 1, 2023
@AndyScherzinger AndyScherzinger added this to the Nextcloud 28 milestone Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants