Move from component to webpack #94

ccyrille · 2019-05-15T08:31:14Z

Executing a make build on the master, even by removing disappeared dependencies like kpwebb/select2, I got the following :

installed : component/querystring@2.0.0 in 495ms
   installed : components/handlebars.js@v4.1.2 in 471ms
   installed : component/trim@0.0.1 in 531ms
   installed : component/type@1.1.0 in 584ms
   installed : es-shims/es5-shim@v4.5.13 in 666ms
   installed : es-shims/es6-shim@0.35.5 in 545ms
   installed : trevorgerhardt/haversine@master in 478ms
     install : complete


       build : resolved in 21ms

fs.js:128
  throw new ERR_INVALID_CALLBACK();
  ^

TypeError [ERR_INVALID_CALLBACK]: Callback must be a function
    at maybeCallback (fs.js:128:9)
    at Object.writeFile (fs.js:1158:14)
    at /Users/cyrille/WORKSPACE/otp.js/node_modules/component/bin/component-build:225:8
    at /Users/cyrille/WORKSPACE/otp.js/node_modules/component-build/lib/styles.js:14:5
    at /Users/cyrille/WORKSPACE/otp.js/node_modules/component-builder/lib/builders/builder.js:142:9
    at next (/Users/cyrille/WORKSPACE/otp.js/node_modules/chanel/lib/proto.js:258:5)
    at Function.onevent (/Users/cyrille/WORKSPACE/otp.js/node_modules/chanel/lib/proto.js:194:9)
    at Object.onceWrapper (events.js:277:13)
    at Function.emit (events.js:189:13)
    at /Users/cyrille/WORKSPACE/otp.js/node_modules/chanel/lib/proto.js:142:10
make: *** [build] Error 1

Problem appears to be at the heart of component package manager, which is not maintained since 2015 (see componentjs/component#639). I decided to migrate the whole project to webpack, sticking as much as possible to the current implementation.

This also solve the following issue : #91

landonreed

Thanks @ccyrille, this looks great and is long overdue! I just had a couple of small nitpicky items regarding whitespace and dependencies.

landonreed · 2019-07-17T13:29:25Z

client/widget.html

+		<script src="https://cdnjs.cloudflare.com/ajax/libs/underscore.js/1.8.3/underscore-min.js"></script>
+		<script src="https://cdnjs.cloudflare.com/ajax/libs/backbone.js/1.2.1/backbone-min.js"></script>
+		<script src="https://cdnjs.cloudflare.com/ajax/libs/leaflet/0.7.7/leaflet-src.js"></script>
+		<script src="https://cdnjs.cloudflare.com/ajax/libs/raphael/2.1.4/raphael-min.js"></script>


Could you fix the whitespace here to be spaces and align with the above script tag?

landonreed · 2019-07-17T13:31:09Z

package.json

+    "inquirer": "^6.3.1",
+    "jquery": "^1.11.3",
+    "leaflet": "^0.7.3",
+    "loadash": "^1.0.0",


Should this be lodash?

You are right, and in fact I wasn't using it anymore so I simply removed it ;)

landonreed · 2019-07-17T13:32:39Z

package.json

+    "haversine": "^1.1.1",
+    "html-loader": "^0.5.5",
+    "inquirer": "^6.3.1",
+    "jquery": "^1.11.3",


A number of these dev dependencies seem to actual dependencies rather than dev dependencies. For example, bootstrap, jquery, backbone, eonasdan-bootstrap-datetimepicker, underscore, moment.

landonreed · 2019-07-17T13:33:35Z

webpack.config.js

+		'window.L': 'leaflet',
+		'$': 'jQuery'
+	}
+};


As with the html file, could you use spaces here instead of tabs to conform with the rest of the project (two spaces per indentation)?

landonreed · 2019-07-17T13:58:45Z

package.json

  },
  "scripts": {
    "start": "serve client",
+    "watch": "webpack --watch",
+    "webpack": "webpack",


The more intuitive name for this webpack script to me is build. It keeps the command build tool-independent and is more of a conventional name. I suppose the Makefile would also need to change accordingly.

ccyrille · 2019-08-06T22:35:18Z

Hi @landonreed, thanks for your nice review ! I agree with all the recommandations you made, and pushed the associated changes. Don't hesitate to come back to me if needed...

ccyrille · 2019-11-22T14:24:50Z

Sorry to bother you @landonreed but any chance you can review this PR ? Do you need me to do additional work from me ?

ccyrille · 2020-01-11T13:48:49Z

Hi @demory @trevorgerhardt, sorry to bother you but I am stuck here. I have put some effort making the otp.js usable / maintainable again by moving to webpack, but I haven't here from Landon since then. Could you help ?

trevorgerhardt · 2020-01-13T09:17:58Z

Hi @ccyrille. We no longer maintain this project as we don't use it internally anymore. It appears the OpenTripPlanner project has mostly moved on from it also with https://github.com/opentripplanner/otp-react-redux and https://github.com/opentripplanner/otp-ui.

The next merged PR should probably be an update relating to the status of this project pointing users to more active client side OTP libs and a more up to date repo for this project.

ccyrille · 2020-01-14T14:50:45Z

Hi @trevorgerhardt, hoping that I didn't work for nothing :

would it be possible to have this PR merged anyway, as it has been fully reviewed and at least it makes the project usable for someone that would want to ? I have remove conflits with master so there is no overhead for you.
could you soon update the README so other people don't face the same problem I did ?

Thanks, Cyrille

ccyrille marked this pull request as ready for review May 15, 2019 10:13

ccyrille changed the title ~~Feature/move to webpack~~ Move from component to webpack May 15, 2019

landonreed suggested changes Jul 17, 2019

View reviewed changes

landonreed reviewed Jul 17, 2019

View reviewed changes

ccyrille added 7 commits January 14, 2020 15:48

Init webpack configuration

d562585

remove unused leg bubble template & css

03ddd2c

move from conponent to webpack

523e9b3

vendor bootstrap-datetimepicker (not iso between npm and cloudflare)

3bb6609

rework Makefile for webpack

98c1dd0

fix datetime pickers config

c571aac

remove unused files

9d5a139

trevorgerhardt merged commit cb50450 into conveyal:master Jan 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move from component to webpack #94

Move from component to webpack #94

ccyrille commented May 15, 2019

landonreed left a comment

landonreed Jul 17, 2019

landonreed Jul 17, 2019

ccyrille Aug 6, 2019

landonreed Jul 17, 2019

landonreed Jul 17, 2019

landonreed Jul 17, 2019 •

edited

Loading

ccyrille commented Aug 6, 2019 •

edited

Loading

ccyrille commented Nov 22, 2019

ccyrille commented Jan 11, 2020

trevorgerhardt commented Jan 13, 2020

ccyrille commented Jan 14, 2020 •

edited

Loading

Move from component to webpack #94

Move from component to webpack #94

Conversation

ccyrille commented May 15, 2019

landonreed left a comment

Choose a reason for hiding this comment

landonreed Jul 17, 2019

Choose a reason for hiding this comment

landonreed Jul 17, 2019

Choose a reason for hiding this comment

ccyrille Aug 6, 2019

Choose a reason for hiding this comment

landonreed Jul 17, 2019

Choose a reason for hiding this comment

landonreed Jul 17, 2019

Choose a reason for hiding this comment

landonreed Jul 17, 2019 • edited Loading

Choose a reason for hiding this comment

ccyrille commented Aug 6, 2019 • edited Loading

ccyrille commented Nov 22, 2019

ccyrille commented Jan 11, 2020

trevorgerhardt commented Jan 13, 2020

ccyrille commented Jan 14, 2020 • edited Loading

landonreed Jul 17, 2019 •

edited

Loading

ccyrille commented Aug 6, 2019 •

edited

Loading

ccyrille commented Jan 14, 2020 •

edited

Loading