jsoverson / preprocess

Preprocess HTML, JavaScript, and other files with directives based off custom or ENV configuration
Other
366 stars 80 forks source link

Include once #65

Open M1ke opened 9 years ago

M1ke commented 9 years ago

Is there currently a system which will include a file exactly once? So if this include appears in a file further along the parsing route (either after or inside the current file) it will be ignored.

If there is not such a directive, would you accept a PR to add one?

BendingBender commented 9 years ago

Hi @M1ke! At the moment, there is no such directive. If you need one and implement it, we will accept your PR but under the following conditions:

M1ke commented 9 years ago

OK thanks. I have now got a basic version of this. It uses the keyword @once to avoid conflicts. So far it has a single test for a basic JS usage case but I'll look to add further tests. Only issue so far is that the recursion of preprocess() isn't encapsulated, meaning that the storage of previously included items has to be manually reset by a controlling process on a per-file basis (i.e. as gulp or grunt parses a directory of master files this would need clearing if the same pp object was used each time).

BendingBender commented 9 years ago

It would be nice for the consistency if you would rename @once into @include-once. Another concern I have is, why you have to manually reset the included files store. Wouldn't it be better to store included files list in a private attribute on the context?

M1ke commented 9 years ago

Sure, I just had an issue with it parsing an include as "-once file.name" so moved to that for ease of the regex.

For the array my issue came because the route stars with .preprocess() which is then called recursively. It seemed therefore that I couldn't set the array inside here, as each recursion would clear it. However this may just be a misunderstanding of how parameters are passed. Is it that the context parameter is consistent through all the recursions? On 18 Jun 2015 20:48, "BendingBender" notifications@github.com wrote:

It would be nice for the consistency if you would rename @once into @include-once. Another concern I have is, why you have to manually reset the included files store. Wouldn't it be better to store included files list in a private attribute on the context?

— Reply to this email directly or view it on GitHub https://github.com/jsoverson/preprocess/issues/65#issuecomment-113270107 .

BendingBender commented 9 years ago

For the rule name, if you want to combine include and include-once (since they are mostly the same), you could change the regex for the simple case to the following: "^(.*)@(include-once|include(?!-))[ \t]*([^\n]*)[ \t]*$" This adds a new capturing group which will contain either "include-once" or "include". You will of course have to adapt processIncludeDirective() to accept the additional matching group and you'll have to do something for the include-static case, which is missing this additional group.

As for the array, the context that is passed on each recursion invocation is either flat copied or is passed as reference. What I'm not sure about is the getFlattedContext() call and whether and how it handles arrays, you'll have to take a look at it.

hellboy81 commented 8 years ago

Can I use @include-once in production?

DanielFlaum commented 2 years ago

I'm interested in reviving this enhancement. It's been several years, though. Do we need to start over from square one?