Problem/Motivation

The core/misc directory is a mess. Lots of random scripts, images, stylesheets, etc. Completely unorganized.

Proposed resolution

  • Move the assets that are defined in system_library_info() that are in core/misc to core/modules/system/assets.

Remaining tasks

  1. #1341792: Ship minified versions of jQuery and jQuery UI since there are some path changes there
  2. Make a new patch for this issue
  3. Review patch
  4. Commit patch
  5. Open the box

User interface changes

No UI changes.

API changes

No API changes since it's handled by hook_library_info().

Original report by _nod

Reusable scripts are currently in /core/misc next to random images and other things.

We need to have a proper structure for putting javascript sources, minified files and third party scripts.

I'm proposing the following structure in core/js

  • src/
  • src/lib/ reusable components like tabledrag
  • dist/ same structure as src/ with minified/uglyfied files
  • vendor third party scripts used in core (like jquery.forms.js, jquery.ba-bbq.js)

this need some more thought, I just want to put it out there. Contrib should also follow the convention.

Files: 
CommentFileSizeAuthor
#62 bower-in-core-1663622-62.diff854 bytesmavimo
#53 core-bower-1663622-53.patch586 bytesnod_
#43 core-js-proper-folder-1663622-42.patch99.87 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 41,980 pass(es).
[ View ]
#42 core-js-proper-folder-1663622-41.patch99.85 KBnod_
FAILED: [[SimpleTest]]: [MySQL] 41,897 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#37 core-js-proper-folder-1663622-37.patch100.22 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 40,694 pass(es).
[ View ]
#35 core-js-proper-folder-1663622-35.patch100.77 KBnod_
FAILED: [[SimpleTest]]: [MySQL] 40,702 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#33 core-js-proper-folder-1663622-33.patch89.47 KBnod_
FAILED: [[SimpleTest]]: [MySQL] 40,525 pass(es), 11 fail(s), and 0 exception(s).
[ View ]
#31 core-js-proper-folder-1663622-32.patch71.55 KBnod_
FAILED: [[SimpleTest]]: [MySQL] 40,526 pass(es), 12 fail(s), and 8 exception(s).
[ View ]
#23 1663622-23.patch29.18 KBsxnc
FAILED: [[SimpleTest]]: [MySQL] 40,235 pass(es), 4 fail(s), and 46 exception(s).
[ View ]
#21 1663622-21.patch29.1 KBsxnc
FAILED: [[SimpleTest]]: [MySQL] 40,241 pass(es), 5 fail(s), and 43 exception(s).
[ View ]
#20 1663622-19.patch895.24 KBsxnc
FAILED: [[SimpleTest]]: [MySQL] 40,241 pass(es), 5 fail(s), and 43 exception(s).
[ View ]
#19 drupal-1663622-19.patch28.68 KBtim.plunkett
FAILED: [[SimpleTest]]: [MySQL] 40,239 pass(es), 5 fail(s), and 45 exception(s).
[ View ]
#18 1663622-18.patch890.31 KBsxnc
FAILED: [[SimpleTest]]: [MySQL] 40,236 pass(es), 5 fail(s), and 45 exception(s).
[ View ]
#17 1663622-17.patch457.31 KBsxnc
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1663622-17.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 core-js-proper-folder-1663622-6.patch113.62 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 36,995 pass(es).
[ View ]
#4 core-js-proper-folder-1663622-4.patch113.1 KBnod_
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-js-proper-folder-1663622-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 core-js-proper-folder-1663622-2.patch113.08 KBnod_
FAILED: [[SimpleTest]]: [MySQL] 36,983 pass(es), 3 fail(s), and 66 exception(s).
[ View ]

Comments

Issue tags:+JavaScript

Regarding third-party vendor code, I'd like to see:

core/vendor
├── cowboy
│   └── jquery-bbq
├── jquery
│   ├── jquery
│   └── jquery-ui
├── malsup
│   └── form
├── symfony
└── twig
    └── twig

Then it would really discourage us from touching those files. At the very least, we should most definitely work more closely with npm and grunt. Those tools will allow us to automate a lot of what we currently do manually.

Status:Active» Needs review
StatusFileSize
new113.08 KB
FAILED: [[SimpleTest]]: [MySQL] 36,983 pass(es), 3 fail(s), and 66 exception(s).
[ View ]

Move all the core/misc JS files, still need to take care of all the modules files.

This will make test fails, just don't know how many :p

Status:Needs review» Needs work

The last submitted patch, core-js-proper-folder-1663622-2.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new113.1 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-js-proper-folder-1663622-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

oups, missed a couple of /js/vendor

That's definitely a good move, and the vendor structure/naming is pretty neat. Thanks for this.

+1 for considering the use of grunt in managing core js.

StatusFileSize
new113.62 KB
PASSED: [[SimpleTest]]: [MySQL] 36,995 pass(es).
[ View ]

haha it passed. need to be verified though

Missed a core/misc,

This is a great move and helps clean up the core/misc folder greatly. Since the System module currently handles all of its assets in core/misc, what are your thoughts on moving those defined in system_library_info() to core/modules/system/assets or something similar? Moving some of core/misc over to modules/system might help decouple it a bit... I'd be willing to do the re-roll.

The discussion of where third-party vendor code should go can continue over at #1473066: [Meta] Move third-party assets out of core/misc folder. The proposed location of it here seems appropriate for this issue.

oh yeah moving that to system would make sense, that way we only have 1 rule about the location of js files.

I have no time to take care of the reroll, please go ahead if you can, thanks.

An issue summary describing which directory structure the patch is trying to do would be helpful. :)

All right, not a serious or very helpful comment, but a funny one: http://xkcd.com/1077/ :)

Status:Needs review» Needs work

Sounds like we want to move them to core/modules/system/assets then. There are some pathings in #1341792: Ship minified versions of jQuery and jQuery UI too. We should probably get that one in first so that updating the locations arn't too too crazy.

Title:Give JS a real folder of his ownChange directory structure for JavaScript files

So what's the current proposal, in total -- could someone update the issue summary?

Issue summary:View changes

contrib

Thanks... Updated the summary.

THANK YOU! I love the first line: "The core/misc directory is a mess. ". Well put!

So I have a couple of questions about this proposal:

a) It seems like many contrib projects have been using the convention that JavaScript files go into a "js" sub-directory. This makes more sense to me than inventing a new directory name "assets" that isn't being used anywhere else (to my knowledge). Is there any particular reason why "assets" is a better idea?

b) This proposal doesn't address other JS files that are in core/misc not covered by system_library_info(). There are a few, I think?

c) In general, because of coding standards not applying to third-party code, it is really helpful to have Drupal vs. third-party code separated into separate directories. I guess this is covered by other issues, but I just mention it here because the proposal in the summary doesn't say that third-party libraries are to be handled differently:
#1473066: [Meta] Move third-party assets out of core/misc folder
#1298304: [Meta] Move all externally-developed files to core/vendor/{vendorname}/ directories

a) It seems like many contrib projects have been using the convention that JavaScript files go into a "js" sub-directory. This makes more sense to me than inventing a new directory name "assets" that isn't being used anywhere else (to my knowledge). Is there any particular reason why "assets" is a better idea?

Projects like jQuery UI are not just limited to JavaScript. They have both JavaScript and stylesheets, hence assets. We could go with another name though. It'll likely be moved out of there via #1473066: [Meta] Move third-party assets out of core/misc folder though. "public" in the Resources namespace is what Symfony uses. Might be good to go with that then.

b) This proposal doesn't address other JS files that are in core/misc not covered by system_library_info(). There are a few, I think?

Likely to be a lot more.

c) In general, because of coding standards not applying to third-party code, it is really helpful to have Drupal vs. third-party code separated into separate directories. I guess this is covered by other issues, but I just mention it here because the proposal in the summary doesn't say that third-party libraries are to be handled differently

Correct. I'll be working on some packaging for external assets that will help with this.

Regarding 'assets' -- OK... Most contrib projects seem to have a separate js and css directory, but I can see the wisdom of combining related JS and CSS so they are together.

Status:Needs work» Needs review
StatusFileSize
new457.31 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1663622-17.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Okay, i made a patch that moves all the .js files from core/misc to core/modules/system/assets. I also modified the JavaScriptTest.php, system.module and core/includes.common.inc to reflect the new paths.

StatusFileSize
new890.31 KB
FAILED: [[SimpleTest]]: [MySQL] 40,236 pass(es), 5 fail(s), and 45 exception(s).
[ View ]

re-submitting patch, previous version did not contain the moved files (it only deleted them).

StatusFileSize
new28.68 KB
FAILED: [[SimpleTest]]: [MySQL] 40,239 pass(es), 5 fail(s), and 45 exception(s).
[ View ]

Rerolled using the configuration detailed here http://drupal.org/node/1542048, specifically git config diff.renames copies
Certainly shrinks the patch size :)

StatusFileSize
new895.24 KB
FAILED: [[SimpleTest]]: [MySQL] 40,241 pass(es), 5 fail(s), and 43 exception(s).
[ View ]

Re-submitt full patch, there were 2 additional files missing: "vertical-tabs-rtl.css" and "vertical-tabs.css".

StatusFileSize
new29.1 KB
FAILED: [[SimpleTest]]: [MySQL] 40,241 pass(es), 5 fail(s), and 43 exception(s).
[ View ]

@tim.plunkett dang it, i didn't refresh when i submitted my patch again... But thanks A LOT for that git rename config thing, we were looking for that! Here comes a much smaller and hopefully issueless patch~

Status:Needs review» Needs work

in common.inc there is still a core/misc/drupal.js that should be renamed as well :)

Once that is fixed and the testbot is happy that's RTBC :)

Status:Needs work» Needs review
StatusFileSize
new29.18 KB
FAILED: [[SimpleTest]]: [MySQL] 40,235 pass(es), 4 fail(s), and 46 exception(s).
[ View ]

@nod_ should be fixed now :) dang it, that was a sloppy mistake...

Great, I'll rtbc after the testbot finishes.

Status:Needs review» Needs work

The last submitted patch, 1663622-23.patch, failed testing.

All right, so core/misc is used all over core since it's not only in library definitions.

So either we wait for the explicit dependencies patch to get in and we can just use this patch. Or we have to replace every call to drupal_add_js that adds a file from this directory.

Status:Needs work» Postponed

Actually there is a lot more depending on this dependencies patch. Postponing as it will be easier afterwards.

Status:Postponed» Needs work

Patch is in, it's on!

So now the only path that needs changing should be in the system.module in system_library_info().

Jam is a package manager for JavaScript. It is an interesting system, but still quite young. It handles downloading the correct versions of packages that you want.

I've talked briefly with Scott González about having jQuery UI support for this. He's open to the idea, but it's not on the immediate radar. Kat Bailey, Crell and many others helped with switching up to Composer to handle our PHP dependencies, would be neat to have something like it handle our JavaScript dependencies too.

Not saying it's something we should do, just something to consider.

I'd open a new issue and set it to postponed if I were you. Just so to make sure that the idea and the specific proposal won't be forgotten.

Status:Needs work» Needs review
StatusFileSize
new71.55 KB
FAILED: [[SimpleTest]]: [MySQL] 40,526 pass(es), 12 fail(s), and 8 exception(s).
[ View ]

nice and easy

Status:Needs review» Needs work

The last submitted patch, core-js-proper-folder-1663622-32.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new89.47 KB
FAILED: [[SimpleTest]]: [MySQL] 40,525 pass(es), 11 fail(s), and 0 exception(s).
[ View ]

All right, forgot the tests :-°

Status:Needs review» Needs work

The last submitted patch, core-js-proper-folder-1663622-33.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new100.77 KB
FAILED: [[SimpleTest]]: [MySQL] 40,702 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Quite a lot of hardcoded paths in tests.

Status:Needs review» Needs work

The last submitted patch, core-js-proper-folder-1663622-35.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new100.22 KB
PASSED: [[SimpleTest]]: [MySQL] 40,694 pass(es).
[ View ]

I don't think that 3rd-party files, such as Farbtastic and jQuery UI, should be going into core/modules/system:

rename from core/misc/farbtastic/farbtastic.css
rename to core/modules/system/assets/farbtastic/farbtastic.css

These really need to be in their own space. Wasn't that the plan?

Well, they don't belong in /misc either and the PHP guys don't want assets in /vendor

So I'm open to putting it anywhere else, just need to know where. We're replication the misc structure in assets, which is not great but way better than the current structure.

How about system/assets/vendor, so that we at least know they're third-party?

StatusFileSize
new99.85 KB
FAILED: [[SimpleTest]]: [MySQL] 41,897 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

reroll. This patch is not using assets/vendor as I'm still unsure if people would OK it.

StatusFileSize
new99.87 KB
PASSED: [[SimpleTest]]: [MySQL] 41,980 pass(es).
[ View ]

was missing one replace in a test.

re #39, I think we need to push back a bit on /core/vendor. Looking ahead to inclusion of libraries like Backbone and Underscore, I'm hesitant to bury them in core/system/assets. We should have all third-party code under a single directory instead of making an arbitrary PHP / non-PHP distinction.

Who objected to /core/vendor? We just need to sit down and chat with them.

Status:Needs review» Postponed

Let's open that back up december 2nd.

Status:Postponed» Needs review

Status:Needs review» Postponed

Didn't want to "needs review". Just was curious if the file locations have changed. Ignore me :-) .

Issue tags:+coding standards

By the way, there's another issue open that is related to this effort:
#1298304: [Meta] Move all externally-developed files to core/vendor/{vendorname}/ directories
(actually it lists this issue in its summary). And this one should have been long ago tagged with "coding standards", so doing that now.

Status:Postponed» Needs review

#4: core-js-proper-folder-1663622-4.patch queued for re-testing.

Status:Needs review» Postponed

sorry, wrong click. ignore me.

Status:Postponed» Needs work

see #44

We should have all third-party code under a single directory instead of making an arbitrary PHP / non-PHP distinction.

Who objected to /core/vendor? We just need to sit down and chat with them.

It was sun and pounard. #1298304-12: [Meta] Move all externally-developed files to core/vendor/{vendorname}/ directories

My stance on this issue is:

/vendor is for PSR-0 namespaced code. If you have PSR-0 namespaced code, put it there. If your code isn't, then you don't.

But the “full discussion” is over at #1473066: [Meta] Move third-party assets out of core/misc folder

Move the assets that are defined in system_library_info() that are in core/misc to core/modules/system/assets

Why not core/libraries and core/libraries/vendor? All of our 3rd-party JS and CSS code are defined in hook_library_info(). I agree with Jesse; burying stuff in core/modules/system/assets is not a good plan.

I think the underlying issue is that it would be very nice if our 3rd party code was easily packaged and bundled with Drupal. Currently, composer only works with PHP code and it is extremely unlikely that it will be adopted for wide-use in front-end-oriented projects. IMO, Bower looks to have the most wide-spread support. What experience do front-end developers have for auto-building projects with 3rd party assets? Do these tools affect our choice of directory structure?

StatusFileSize
new586 bytes

There is already a lib/ directory so libraries/ would look silly. vendor would still be the best. Here is a couple of bower files that gets most of our dependencies.

Just apply, install bower and run bower install. Really neat.

An on the hook_library_info() topic (I'll open a new issue, it's just that there are more people following this one for the moment). How about declaring assets in an MYMODULE.assets.yml file instead of a hook_library_info()? I got scoled by sun a while back because there shouldn't be anything dynamic in the library_info hook, so it'd make sense. Would address #1969916-5: Remove drupal_add_js/css from seven.theme too.

( edit ) and here is the issue #1996238: Replace hook_library_info() by *.libraries.yml file.

Forgive me if I missed something, but have we thought about just /core/assets? It’s not buried, and we could choose (sensibly) to put 3rd-party code in either core/assets/vendor or core/vendor/assets :)

Here is a sketch of what this might look like:

core/
  assets/
    vendor/
      backbone/
      ckeditor/
      jquery-ui/
      modernizr/
      normalize/
      underscore/
    css/
      base.css <- SMACSS base styles for core
    js/
      ajax.js
      announce.js
    images/
    fonts/
    components/ <- components define complex ui objects & bundle js, css, images…
      collapse
        collapse.js
      utilities/
        utilities.layout.css <- .container-inline, etc.
        utilities.accessibility.css <- .element-invisible, etc.
        utilities.js.css <- JS util classes like .js-show
      buttons/
        css/
          buttons.css <- only essential styles; themes implement look-and-feel
      progress/
        css/
          progress.css
        js/
          jquery.progress.js

core/vendor/assets has been vehemently argued against. *sigh* I say let's just go with core/assets and core/assets/vendor

I'm playing with nod_'s patch in #53 to see if I can get the 3rd party vendor assets to organize themselves into the correct directory. :-)

I've started a branch moving stuff over in the mobile sandbox. http://drupalcode.org/sandbox/johnalbin/1488942.git/shortlog/refs/heads/...

All of the vendor libraries I've looked so far seem to have Bower registration (even if they aren't working), so I'm using those Bower registrations to determine which directory under core/assets/vendor they should go under. e.g. normalize is going under core/assets/vendor/normalize-css

I realized I was about to upload a patch to the wrong issue. I've uploaded a patch to #1473066-24: [Meta] Move third-party assets out of core/misc folder that moves all the 3rd party vendor code from core/misc to core/assets/vendor.

I think the remaining core/misc/*.js files should be moved to core/assets/js/*.js.

Do we still need this issue now that #1473066: [Meta] Move third-party assets out of core/misc folder has been taken care of?

As I said before: I think the remaining core/misc/*.js files should be moved to core/assets/js/*.js.

I'd like to move everything out of core/misc and into more structured folders.

Having the 3rd party libraries in core/assets/vendor, I believe, is the best separation we can get from other CSS/JS stuff given the rest of our directory structure.

umm if you look at drop button you have CSS and js files. would be good to keep that grouped.

RE: #60 Agree. In #54 I suggested core/assets/components for this purpose. Here’s a more readable version:

core/
  assets/
    vendor/
    css/
    js/
    components/
      dropbutton/
      …
    fonts/
    images/

StatusFileSize
new854 bytes

Patch to add bower as manager to external libraries.

NOTE:

  • Some libraries are unmantained (jquery-bbq last commit is 3years ago, come PR are available on GithHub but are ignored, ..)
  • Some libraries require change in path (eg: jquery.ui need to be jquery-ui, html5shiv need to be html5shiv/src, ..)

If this solution is approached I can work on required changes in PHP code.

+1 to the structure proposed by ry5n in #61.

Issue summary:View changes

Updated issue summary.