mozilla / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
176 stars 278 forks source link

Replace extension dependencies in `node_modules` with submodule equivalents #324

Closed humphd closed 9 years ago

humphd commented 9 years ago

Is the Travis/Gruntfile build stuff not doing the right thing, and missing deps?

GET https://mozillathimblelivepreview.net/bramble/node_modules/slowparse/locale/en_US.json 404
GET "https://mozillathimblelivepreview.net/bramble/node_modules/slowparse/locale/en_US.json".text.get
[Extension] failed to load /bramble/dist/extensions/extra/HTMLHinter - Error: /bramble/dist/extensions/extra/HTMLHinter/../../../../node_modules/slowparse/locale/en_US.json HTTP status: 404
humphd commented 9 years ago

NOTE: this was loading the following URL: https://bramble.mofostaging.net/?enableExtensions=HTMLHinter

humphd commented 9 years ago

Seems to work fine locally in both src/ and /dist builds.

humphd commented 9 years ago

Two problems:

To get the node_modules/ into dist/ you can do this:

diff --git a/Gruntfile.js b/Gruntfile.js
index 9cc665b..405b584 100644
--- a/Gruntfile.js
+++ b/Gruntfile.js
@@ -150,6 +150,15 @@ module.exports = function (grunt) {
                             'thirdparty/text/*.js'
                         ]
                     },
+                    /* add node_modules from the root */
+                    {
+                        expand: true,
+                        dest: 'dist/',
+                        cwd: './',
+                        src: [
+                            'node_modules/**/*'
+                        ]
+                    },
                     /* styles, fonts and images */
                     {
                         expand: true,

Getting the paths right is left to the reader.

sedge commented 9 years ago

@humphd We see the problem in deployment when accessing /dist, but it would exist for /src too. Basically, if we are copying node_modules then we have to copy it to both /src and /dist in order to write the paths once and have them work in both places.

I propose that we move third-party extension dependencies to src/thirdparty using submodules, or copying stuff wholesale, so that we skip this problem entirely. That would make sure any extension using that dep would load it from the same place.

Alternatively, it would be nice to use npm. That would require creating a grunt task to install those deps from a package.json file in src/thirdparty.

What do you think?

humphd commented 9 years ago

Agree, make it work using src/thirdparty and submodules. That's the Brackets way.