Skip to content

refactor(@angular/cli): remove deploy URL option and CSS special cases #9910

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/documentation/angular-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
- *root* (`string`): The root directory of the app.
- *outDir* (`string`): The output directory for build results. Default is `dist/`.
- *assets* (`array`): List of application assets.
- *deployUrl* (`string`): URL where files will be deployed.
- *index* (`string`): The name of the start HTML file. Default is `index.html`
- *main* (`string`): The name of the main entry-point file.
- *polyfills* (`string`): The name of the polyfills entry-point file. Loaded before the app.
Expand Down
5 changes: 0 additions & 5 deletions packages/@angular/cli/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,6 @@ export default class BuildCommand extends Command {
}

public async run(options: BuildTaskOptions) {
// Add trailing slash if missing to prevent https://github.com/angular/angular-cli/issues/7295
if (options.deployUrl && options.deployUrl.substr(-1) !== '/') {
options.deployUrl += '/';
}

const BuildTask = require('../tasks/build').default;

const buildTask = new BuildTask({
Expand Down
4 changes: 0 additions & 4 deletions packages/@angular/cli/lib/config/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,6 @@
},
"default": []
},
"deployUrl": {
"type": "string",
"description": "URL where files will be deployed."
},
"baseHref": {
"type": "string",
"description": "Base url for the application being built."
Expand Down
1 change: 0 additions & 1 deletion packages/@angular/cli/models/build-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export interface BuildOptions {
vendorChunk?: boolean;
commonChunk?: boolean;
baseHref?: string;
deployUrl?: string;
verbose?: boolean;
progress?: boolean;
i18nFile?: string;
Expand Down
1 change: 0 additions & 1 deletion packages/@angular/cli/models/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export class NgCliWebpackConfig<T extends BuildOptions = BuildOptions> {
public mergeConfigs(buildOptions: T, appConfig: any, projectRoot: string): T {
const mergeableOptions: Partial<BuildOptions> = {
outputPath: path.resolve(projectRoot, appConfig.outDir),
deployUrl: appConfig.deployUrl,
baseHref: appConfig.baseHref
};

Expand Down
1 change: 0 additions & 1 deletion packages/@angular/cli/models/webpack-configs/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ export function getBrowserConfig(wco: WebpackConfigOptions) {
output: appConfig.index,
baseHref: buildOptions.baseHref,
entrypoints: generateEntryPoints(appConfig),
deployUrl: buildOptions.deployUrl,
}),
]),
node: false,
Expand Down
1 change: 0 additions & 1 deletion packages/@angular/cli/models/webpack-configs/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ export function getCommonConfig(wco: WebpackConfigOptions) {
entry: entryPoints,
output: {
path: path.resolve(buildOptions.outputPath),
publicPath: buildOptions.deployUrl,
filename: `[name]${hashFormat.chunk}.js`,
},
performance: {
Expand Down
22 changes: 1 addition & 21 deletions packages/@angular/cli/models/webpack-configs/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
const hashFormat = getOutputHashFormat(buildOptions.outputHashing);
// Convert absolute resource URLs to account for base-href and deploy-url.
const baseHref = wco.buildOptions.baseHref || '';
const deployUrl = wco.buildOptions.deployUrl || '';

const postcssPluginCreator = function(loader: webpack.loader.LoaderContext) {
return [
Expand Down Expand Up @@ -99,24 +98,6 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
}
}),
postcssUrl([
{
// Only convert root relative URLs, which CSS-Loader won't process into require().
filter: ({ url }: PostcssUrlAsset) => url.startsWith('/') && !url.startsWith('//'),
url: ({ url }: PostcssUrlAsset) => {
if (deployUrl.match(/:\/\//) || deployUrl.startsWith('/')) {
// If deployUrl is absolute or root relative, ignore baseHref & use deployUrl as is.
return `${deployUrl.replace(/\/$/, '')}${url}`;
} else if (baseHref.match(/:\/\//)) {
// If baseHref contains a scheme, include it as is.
return baseHref.replace(/\/$/, '') +
`/${deployUrl}/${url}`.replace(/\/\/+/g, '/');
} else {
// Join together base-href, deploy-url and the original URL.
// Also dedupe multiple slashes into single ones.
return `/${baseHref}/${deployUrl}/${url}`.replace(/\/\/+/g, '/');
}
}
},
{
// TODO: inline .cur if not supporting IE (use browserslist to check)
filter: (asset: PostcssUrlAsset) => {
Expand All @@ -130,7 +111,6 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
{ url: 'rebase' },
]),
PostcssCliResources({
deployUrl: loader.loaders[loader.loaderIndex].options.ident == 'extracted' ? '' : deployUrl,
loader,
filename: `[name]${hashFormat.file}.[ext]`,
}),
Expand All @@ -146,7 +126,7 @@ export function getStylesConfig(wco: WebpackConfigOptions) {
'postcss-url': 'postcssUrl',
'postcss-import': 'postcssImports',
},
variables: { hashFormat, baseHref, deployUrl, projectRoot, maximumInlineSize }
variables: { hashFormat, baseHref, projectRoot, maximumInlineSize }
};

// use includePaths from appConfig
Expand Down
5 changes: 2 additions & 3 deletions packages/@angular/cli/tasks/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const SilentError = require('silent-error');
const opn = require('opn');
const yellow = chalk.yellow;

function findDefaultServePath(baseHref: string, deployUrl: string): string | null {
function findDefaultServePath(baseHref: string, deployUrl = ''): string | null {
if (!baseHref && !deployUrl) {
return '';
}
Expand Down Expand Up @@ -73,7 +73,6 @@ export default Task.extend({
}

const serveDefaults = {
deployUrl: appConfig.deployUrl || '',
baseHref: appConfig.baseHref,
};

Expand Down Expand Up @@ -198,7 +197,7 @@ export default Task.extend({
let servePath = serveTaskOptions.servePath;
if (!servePath && servePath !== '') {
const defaultServePath =
findDefaultServePath(serveTaskOptions.baseHref, serveTaskOptions.deployUrl);
findDefaultServePath(serveTaskOptions.baseHref);
const showWarning = CliConfig.fromProject().get('warnings.servePathDefault');
if (defaultServePath == null && showWarning) {
ui.writeLine(oneLine`
Expand Down
54 changes: 8 additions & 46 deletions tests/e2e/tests/build/css-urls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,58 +48,20 @@ export default function () {
.then(() => expectToFail(() => expectFileToExist('dist/component-img-absolute.svg')))
.then(() => expectFileMatchToExist('./dist', /global-img-relative\.[0-9a-f]{20}\.png/))
.then(() => expectFileMatchToExist('./dist', /component-img-relative\.[0-9a-f]{20}\.png/))
// Check urls with deploy-url scheme are used as is.
.then(() => ng('build', '--base-href=/base/', '--deploy-url=http://deploy.url/',
'--extract-css'))
// Check urls with base-href scheme are used as is.
.then(() => ng('build', '--base-href=http://base.url/', '--extract-css'))
.then(() => expectFileToMatch('dist/styles.css',
/url\(\'http:\/\/deploy\.url\/assets\/global-img-absolute\.svg\'\)/))
/url\(\'\/assets\/global-img-absolute\.svg\'\)/))
.then(() => expectFileToMatch('dist/main.js',
/url\(\'http:\/\/deploy\.url\/assets\/component-img-absolute\.svg\'\)/))
// Check urls with base-href scheme are used as is (with deploy-url).
.then(() => ng('build', '--base-href=http://base.url/', '--deploy-url=deploy/',
'--extract-css'))
/url\(\'\/assets\/component-img-absolute\.svg\'\)/))
// Check with base-href.
.then(() => ng('build', '--base-href=/base/', '--extract-css', '--aot'))
.then(() => expectFileToMatch('dist/styles.css',
/url\(\'http:\/\/base\.url\/deploy\/assets\/global-img-absolute\.svg\'\)/))
.then(() => expectFileToMatch('dist/main.js',
/url\(\'http:\/\/base\.url\/deploy\/assets\/component-img-absolute\.svg\'\)/))
// Check urls with deploy-url and base-href scheme only use deploy-url.
.then(() => ng('build', '--base-href=http://base.url/', '--deploy-url=http://deploy.url/',
'--extract-css'))
.then(() => expectFileToMatch('dist/styles.css',
/url\(\'http:\/\/deploy\.url\/assets\/global-img-absolute\.svg\'\)/))
.then(() => expectFileToMatch('dist/main.js',
/url\(\'http:\/\/deploy\.url\/assets\/component-img-absolute\.svg\'\)/))
// Check with base-href and deploy-url flags.
.then(() => ng('build', '--base-href=/base/', '--deploy-url=deploy/',
'--extract-css', '--aot'))
.then(() => expectFileToMatch('dist/styles.css',
'/base/deploy/assets/global-img-absolute.svg'))
'/assets/global-img-absolute.svg'))
.then(() => expectFileToMatch('dist/styles.css',
/global-img-relative\.[0-9a-f]{20}\.png/))
.then(() => expectFileToMatch('dist/main.js',
'/base/deploy/assets/component-img-absolute.svg'))
.then(() => expectFileToMatch('dist/main.js',
/deploy\/component-img-relative\.[0-9a-f]{20}\.png/))
// Check with identical base-href and deploy-url flags.
.then(() => ng('build', '--base-href=/base/', '--deploy-url=/base/',
'--extract-css', '--aot'))
.then(() => expectFileToMatch('dist/styles.css',
'/base/assets/global-img-absolute.svg'))
.then(() => expectFileToMatch('dist/styles.css',
/global-img-relative\.[0-9a-f]{20}\.png/))
.then(() => expectFileToMatch('dist/main.js',
'/base/assets/component-img-absolute.svg'))
.then(() => expectFileToMatch('dist/main.js',
/\/base\/component-img-relative\.[0-9a-f]{20}\.png/))
// Check with only base-href flag.
.then(() => ng('build', '--base-href=/base/',
'--extract-css', '--aot'))
.then(() => expectFileToMatch('dist/styles.css',
'/base/assets/global-img-absolute.svg'))
.then(() => expectFileToMatch('dist/styles.css',
/global-img-relative\.[0-9a-f]{20}\.png/))
.then(() => expectFileToMatch('dist/main.js',
'/base/assets/component-img-absolute.svg'))
'/assets/component-img-absolute.svg'))
.then(() => expectFileToMatch('dist/main.js',
/component-img-relative\.[0-9a-f]{20}\.png/));
}
41 changes: 0 additions & 41 deletions tests/e2e/tests/build/deploy-url.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,24 @@ export default function () {
return Promise.resolve()
.then(() => writeMultipleFiles({
'src/string-script.js': 'console.log(\'string-script\'); var number = 1+1;',
})
}))
.then(() => updateJsonFile('.angular-cli.json', configJson => {
configJson['apps'][0]['scripts'] = [
'string-script.js',
];
}))
// check when setup through command line arguments
.then(() => ngServe('--deploy-url', '/deployurl/', '--base-href', '/deployurl/'))
.then(() => ngServe('--base-href', '/nested/'))
.then(() => request('http://localhost:4200'))
.then(body => {
if (!body.match(/<app-root><\/app-root>/)) {
throw new Error('Response does not match expected value. (1)');
}
if (!body.match(/"\/deployurl\/scripts.js"/)) {
if (!body.match(/"scripts.js"/)) {
throw new Error('Response does not match expected value. (2)');
}
})
.then(() => request('http://localhost:4200/deployurl/'))
.then(() => request('http://localhost:4200/nested/'))
.then(body => {
if (!body.match(/<app-root><\/app-root>/)) {
throw new Error('Response does not match expected value. (3)');
Expand Down