Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make develop build and run using normal npm conventions #7305

Closed
ara4n opened this issue Sep 9, 2018 · 6 comments
Closed

Make develop build and run using normal npm conventions #7305

ara4n opened this issue Sep 9, 2018 · 6 comments
Assignees
Labels
P1 T-Enhancement T-Task Tasks for the team like planning

Comments

@ara4n
Copy link
Member

ara4n commented Sep 9, 2018

Rather than messing with fetch-develop.deps.sh / mandating manual linked checkouts, or having to run three separate npm run start processes, it would be much more simpler if riot-web just let you npm run build or npm run start on develop to pull in the right versions of react-sdk and js-sdk as per package.json. Users could then npm link if they want to customise a particular version of the dep, but would ever only need a single npm run start instance.

@ara4n
Copy link
Member Author

ara4n commented Sep 9, 2018

in practice, i think this means:

  • adding post-install scripts to js-sdk and react-sdk which make them build their /lib dirs after install (and checking it doesn't break CI or release processes), so that if npm pulls them in from github, they can still be used as real dependencies without having to manually npm run build them.
  • making npm run start also somehow safely cascade into node_modules and recurse onto js-sdk and react-sdk too.

The end result would be that you could do:

git clone git+ssh://git@github.com/vector-im/riot-web
git checkout develop
cd riot-web
npm i
npm run start # or npm run build

...and it would 'do the right thing' and work.

@martindale, does that match your expectations?

@martindale
Copy link

martindale commented Sep 9, 2018

@ara4n yes, this is great! Thanks for taking the time to write this up. I think it'll help a lot — if I have time this week, I'm going to take a crack at shoring up the dependency tree and making this work.

@bwindels
Copy link
Contributor

I'd love our build setup to be simpler as well, but I wonder how js-sdk and react-sdk would get built after making local changes if you've set it up with npm link to come from some other local directory.

We might be able to configure webpack to monitor node_modules/matrix-js|react-sdk as well from riot-web, as described here: https://stackoverflow.com/questions/41522721/how-to-watch-certain-node-modules-changes-with-webpack-dev-server/44166532#44166532

@bwindels
Copy link
Contributor

That approach seems less like npm conventions to me though (where every module is supposed to build itself), but taking this shortcut for local development only might be worth the convenience.

@ara4n
Copy link
Member Author

ara4n commented Sep 10, 2018

I've started a google doc to try to gather and comment on options here: https://docs.google.com/document/d/138w2bb8BokVR6OJA7_5qIeDFkjp9GODzlrqhVC9sMEI/edit#

@ara4n ara4n added feature T-Task Tasks for the team like planning P1 labels Sep 10, 2018
turt2live added a commit that referenced this issue Sep 17, 2018
A step towards a real solution for #7305

This approach makes use of `npm link` to remove the use of symlinks in the build process. The build process has also been altered to invoke the build process of each underlying SDK (react, js). This means that one can now `npm link` and `npm start` and have a working environment. 

At the same time, parallelshell was dropped due to lack of maintenance from the maintainer.
@turt2live
Copy link
Member

This is mostly fixed by #7355 - we still have a couple oddities, although they seem unavoidable given the requirement to maintain a js-sdk and react-sdk separate to riot-web.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 T-Enhancement T-Task Tasks for the team like planning
Projects
None yet
Development

No branches or pull requests

5 participants