feat: implement --base-href argument #1506

dzonatan · 2016-07-31T20:17:22Z

Implement --base-href argument for build and github-pages:deploy commands.
Works as described in this comment.

One thing I did not fulfil from @filipesilva comment is:

the github deploy feature already does a regex base replacement, so it needs to be changed to use this feature

Everything I found was "updateBaseHref" method which uses this /<base href="\/">/g regex. After investigation it turns out that it will only replace the current <base href="/"> (it must be declared this way in your index.html otherwise it won't work) tag. Also I don't get why regex g modifier is used when base tag can be only one per page. Therefore I made my own regex to meet the insert whole tag (if not exists) & replace only href (if exists) requirements.

frapontillo · 2016-08-17T09:48:30Z

Awesome work! Can't wait to see it merged in.

filipesilva · 2016-08-23T02:09:12Z

@dzonatan my apologies, I took too long to review this PR and meanwhile there were some big changes in the files you edited. I looked through your test suite and it looks good. Can you rebase to the latest master?

Also, can you add the --base-href option to this command, and pass on the option to build? https://github.com/angular/angular-cli/blob/master/addon/ng2/commands/github-pages-deploy.ts

dzonatan · 2016-08-23T08:04:41Z

Done. Also added -bh alias for --base-href if you don't mind.

filipesilva · 2016-08-24T16:22:11Z

README.md

+```bash
+# Sets base tag href to /myUrl/ in your index.html
+ng build --base-href /myUrl/
+ng serve --base-href /myUrl/


Does it make sense to have this flag in the serve command? I'd expect the app to not work since the Dev server isn't aware of it. Haven't tested though.

Heh. You're right! The given example will not work as expected.
I was just thinking about the opposite case: when using production value in index.html and you need to override it back to "/" when developing. But seems that would be the only working case. So I will remove it form serve and rebase master.

filipesilva · 2016-08-24T16:22:50Z

Left a question but otherwise lgtm!

dzonatan · 2016-08-24T18:09:58Z

@filipesilva updated according to your comments

filipesilva · 2016-08-24T21:44:45Z

Ah there's something else that I remembered now: the github deploy command does it's own base tag replacement:

https://github.com/dzonatan/angular-cli/blob/8dd2cfe53e90ddffa308eceb444b973d05f61331/addon/ng2/commands/github-pages-deploy.ts#L207-L216

This logic needs to go into the build options now. Basically, if options.userPage is true, then we don't auto-replace the tag. Otherwise, it needs to be replaced with the project name since that's the base for github pages projects.

You'll probably want to have the --base-href flag take priority over that logic though, in case people want to force a certain tag.

Also, these tests should now not try and test the base href functionality, since that's part of the build task now (and I can't imagine a good way to test it from within this test anymore):
https://github.com/dzonatan/angular-cli/blob/8dd2cfe53e90ddffa308eceb444b973d05f61331/tests/acceptance/github-pages-deploy.spec.js#L86

There's a few instances of this check mind you.

dzonatan · 2016-08-25T08:25:18Z

I'm not the expert of github pages so here some questions:

As I understood from fix(gh-deploy): fix deep links #1020 PR this line in updateBaseHref is still needed and should be left as separate deployment step. Right?
Should this line in printProjectUrl be somehow modified according to --base-href logic?

Anyway, I pushed a new commit for easier review. Please look at it.

dzonatan · 2016-08-25T08:30:28Z

addon/ng2/tasks/build-webpack-watch.ts


 let lastHash: any = null;

 module.exports = Task.extend({
-  run: function(runTaskOptions: ServeTaskOptions) {
+  run: function(runTaskOptions: BuildOptions) {


Strange, that nobody has noticed yet that build-webpack and build-webpack-watch tasks uses the options interface from serve command. 😕

Looks like one of those things that slipped :/

filipesilva · 2016-08-25T16:43:59Z

You're absolutely right, this still needs to happen. Maybe you can just replace the updateBaseHref function with a add404Page function, that just copies index.html to 404.html in the same position in the pipeline.
Hm... I don't think so. The case where you manually change the baseUrl for a GitHub deploy is when you have it on a custom domain or something. We can't know what the custom domain is atm, so might as well leave this bit as is.

filipesilva · 2016-08-25T16:45:46Z

Just noticed that had already done what I suggested via the createNotFoundPage function, cheers!

filipesilva · 2016-08-25T16:48:06Z

Reviewed and flagged it as LGTM. It will be merged soon to be on the next release.

Thank you so much for this feature!

dzonatan · 2016-08-25T17:18:44Z

Cool! But I see new conflicts.. again. 😄 Do I need to solve them?

filipesilva · 2016-08-25T20:19:26Z

They're probably lint related, since I a big linting PR just got pushed. If you can look at them I'd appreciate it, but I expect it to be minor stuff as far as your PR is concerned.

Implement --base-href argument for build and github-pages:deploy commands Closes angular#1064

Reuse basehref handling logic from webpack build task

dzonatan · 2016-08-26T05:43:32Z

Forgot to notify that I rebased it yesterday.

filipesilva · 2016-08-26T15:36:57Z

It's in! Thanks @dzonatan for all the work you put in this functionality, you should be able to use it in the next webpack release :D

angular-automatic-lock-bot · 2019-09-10T03:41:34Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

With this change we add hybrid rendering support in common engine, express engine and hapi engine. Users don't need to do any changes to take advantage of this change. The engine will serve the static prerendered page if a index.html exists for the given route, otherwise it will fallback to render the page in SSR. Closes angular#1506

googlebot added the cla: yes label Jul 31, 2016

dzonatan force-pushed the webpack-feature-basehref branch from ea99974 to f9e08c7 Compare July 31, 2016 20:58

dzonatan force-pushed the webpack-feature-basehref branch from f9e08c7 to b216b06 Compare August 9, 2016 07:37

filipesilva added the pr_action: review label Aug 10, 2016

filipesilva added state: WIP and removed pr_action: review labels Aug 23, 2016

dzonatan force-pushed the webpack-feature-basehref branch from b3925fc to 35413c6 Compare August 23, 2016 08:03

filipesilva added pr_action: review and removed state: WIP labels Aug 23, 2016

filipesilva reviewed Aug 24, 2016
View reviewed changes

dzonatan force-pushed the webpack-feature-basehref branch from 35413c6 to 8dd2cfe Compare August 24, 2016 17:48

dzonatan force-pushed the webpack-feature-basehref branch from 8dd2cfe to c159363 Compare August 25, 2016 08:22

dzonatan reviewed Aug 25, 2016
View reviewed changes

filipesilva added pr_state: LGTM and removed pr_action: review labels Aug 25, 2016

feat: implement --base-href argument

e6dc4c9

Implement --base-href argument for build and github-pages:deploy commands Closes angular#1064

dzonatan force-pushed the webpack-feature-basehref branch 2 times, most recently from 49df2da to 1e1ef93 Compare August 25, 2016 21:50

refactor(github-pages-deploy): reuse webpack build --base-href logic

f94a022

Reuse basehref handling logic from webpack build task

dzonatan force-pushed the webpack-feature-basehref branch from 1e1ef93 to f94a022 Compare August 25, 2016 21:52

filipesilva closed this in 74b29b3 Aug 26, 2016

dzonatan deleted the webpack-feature-basehref branch September 12, 2016 09:50

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement --base-href argument #1506

feat: implement --base-href argument #1506

dzonatan commented Jul 31, 2016 •

edited

Loading

frapontillo commented Aug 17, 2016

filipesilva commented Aug 23, 2016

dzonatan commented Aug 23, 2016

filipesilva Aug 24, 2016

dzonatan Aug 24, 2016

filipesilva commented Aug 24, 2016

dzonatan commented Aug 24, 2016

filipesilva commented Aug 24, 2016

dzonatan commented Aug 25, 2016

dzonatan Aug 25, 2016

filipesilva Aug 25, 2016

filipesilva commented Aug 25, 2016

filipesilva commented Aug 25, 2016

filipesilva commented Aug 25, 2016

dzonatan commented Aug 25, 2016

filipesilva commented Aug 25, 2016

dzonatan commented Aug 26, 2016

filipesilva commented Aug 26, 2016

angular-automatic-lock-bot bot commented Sep 10, 2019

feat: implement --base-href argument #1506

feat: implement --base-href argument #1506

Conversation

dzonatan commented Jul 31, 2016 • edited Loading

frapontillo commented Aug 17, 2016

filipesilva commented Aug 23, 2016

dzonatan commented Aug 23, 2016

filipesilva Aug 24, 2016

Choose a reason for hiding this comment

dzonatan Aug 24, 2016

Choose a reason for hiding this comment

filipesilva commented Aug 24, 2016

dzonatan commented Aug 24, 2016

filipesilva commented Aug 24, 2016

dzonatan commented Aug 25, 2016

dzonatan Aug 25, 2016

Choose a reason for hiding this comment

filipesilva Aug 25, 2016

Choose a reason for hiding this comment

filipesilva commented Aug 25, 2016

filipesilva commented Aug 25, 2016

filipesilva commented Aug 25, 2016

dzonatan commented Aug 25, 2016

filipesilva commented Aug 25, 2016

dzonatan commented Aug 26, 2016

filipesilva commented Aug 26, 2016

angular-automatic-lock-bot bot commented Sep 10, 2019

dzonatan commented Jul 31, 2016 •

edited

Loading