Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[TwigBundle] Cache Warmer node_modules bug fixed#34321

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
cesurapp wants to merge1 commit intosymfony:4.3fromcesurapp:4.3
Closed

[TwigBundle] Cache Warmer node_modules bug fixed#34321

cesurapp wants to merge1 commit intosymfony:4.3fromcesurapp:4.3

Conversation

@cesurapp
Copy link
Contributor

QA
Branch?4.3 bug fixes
Bug fix?yes
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

I created a theme structure in the "templates" directory. A different webpack structure is used for each theme.

I get an error while clearing the Symfony cache "bin/console cache:clear". While the cache is warming up, twig is trying to read files in node_modules directory.

There is no problem with "bin/console cache:clear --no-warmup".

Error:

PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 815104 bytes) in /Users/apaydin/www/emlakpro/apps/vendor/twig/twig/src/Compiler.php on line 129

dmaicher, derrabus, and stloyd reacted with confused emojicesurapp reacted with heart emoji
@cesurappcesurapp changed the titleCache Warmer node_modules bug fixed[TwigBundle] Cache Warmer node_modules bug fixedNov 11, 2019
@nicolas-grekasnicolas-grekas added this to the4.3 milestoneNov 12, 2019
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

The name is specific enough, I think we can do it.

Copy link
Member

@ycerutoyceruto left a comment
edited
Loading

Choose a reason for hiding this comment

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

I'm 👎 on this. I don't think this is the right approach (there are a lot of well known directories that could be applied here) and I'm pretty sure you can solve this problem by moving the folder out oftemplates/ in the meantime.

The right approach in my opinion would be adding a new option to exclude directories (e.g.bundles/ by default) and/or files, using some kind of glob/pattern.

See dicussion about this topic here#34142

@nicolas-grekas
Copy link
Member

there are a lot of well known directories that could be applied here

I'm not sure about this - I mean:node_modules is very very well-known. I can't recall any other directory that well known. That makes it special to me.

I'm pretty sure you can solve this problem by moving the folder out of templates/ in the meantime.

@appaydin please tell us why you didn't do this in the first place?

The right approach in my opinion would be adding a new option to exclude directories (e.g. bundles/ by default) and/or files, using some kind of glob/pattern.

That's a lot of additional complexity for a rare case. Excluding justnode_modules would save us from that complexity. That's asignificant benefit.

#34142 is far from being solved at all: "non-template files" is undefined. I wouldn't consider this a viable alternative solution in the near term...

@yceruto
Copy link
Member

I agree, I would also consider this a rare case, that's why I'm against to add more directories arbitrarily to the hardcoded exclusion list and go ahead with workarounds.

I still don't think this use case (having thenode_modules/ inside the Twig'stemplates/ directory) is enough legit and common to exclude it.

@yceruto
Copy link
Member

Although I don't feel comfortable telling people where they should put their directories/files due to a problem generated by the framework. That's why also I would want do something about it.

@cesurapp
Copy link
ContributorAuthor

Keeping the front-end of the site in a single directory does not disturb the theme structure. There are different themes created with Vue, and you cannot move the node_modules directory to a different location.

In order to get node_modules into a different directory, we have to split the theme structure into two.

Alternative options are possible, but the directory structure becomes more complex.

https://stackoverflow.com/questions/21818701/can-a-custom-directory-name-be-used-instead-of-node-modules-when-installing-no

@yceruto
Copy link
Member

The right approach in my opinion would be adding a new option to exclude directories (e.g. bundles/ by default) and/or files, using some kind of glob/pattern.

That's a lot of additional complexity for a rare case. Excluding just node_modules would save us from that complexity. That's a significant benefit.

Please, let's discuss it here#34416, I don't think there's much additional complexity to it.

@fabpot
Copy link
Member

👎 for me

@nicolas-grekas
Copy link
Member

Closing then, thanks for proposing @appaydin

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@ycerutoycerutoyceruto requested changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

5 participants

@cesurapp@nicolas-grekas@yceruto@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp