Skip to content

Commit

Permalink
refactor: remove safari nomodule polyfills as it's unsupported
Browse files Browse the repository at this point in the history
(cherry picked from commit d0ede14)
  • Loading branch information
alan-agius4 authored and dgp1130 committed Feb 11, 2020
1 parent 3939142 commit 07d0879
Show file tree
Hide file tree
Showing 8 changed files with 6 additions and 184 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -132,19 +132,6 @@ export function getCommonConfig(wco: WebpackConfigOptions): Configuration {
buildOptions.es5BrowserSupport ||
(buildOptions.es5BrowserSupport === undefined && buildBrowserFeatures.isEs5SupportNeeded())
) {
// The nomodule polyfill needs to be inject prior to any script and be
// outside of webpack compilation because otherwise webpack will cause the
// script to be wrapped in window["webpackJsonp"] which causes this to fail.
if (buildBrowserFeatures.isNoModulePolyfillNeeded()) {
const noModuleScript: ExtraEntryPoint = {
bundleName: 'polyfills-nomodule-es5',
input: path.join(__dirname, '..', 'safari-nomodule.js'),
};
buildOptions.scripts = buildOptions.scripts
? [...buildOptions.scripts, noModuleScript]
: [noModuleScript];
}

const polyfillsChunkName = 'polyfills-es5';
entryPoints[polyfillsChunkName] = [path.join(__dirname, '..', 'es5-polyfills.js')];
if (differentialLoadingMode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,17 +148,17 @@ export async function augmentIndexHtml(params: AugmentIndexHtmlOptions): Promise
// such as runtime.js
const scriptPredictor = ({ file }: FileInfo): boolean => file === script;
if (!files.some(scriptPredictor)) {
// in some cases for differential loading file with the same name is avialable in both
// in some cases for differential loading file with the same name is available in both
// nomodule and module such as scripts.js
// we shall not add these attributes if that's the case
const isNoModuleType = noModuleFiles.some(scriptPredictor);
const isModuleType = moduleFiles.some(scriptPredictor);

if (isNoModuleType && !isModuleType) {
attrs.push({ name: 'nomodule', value: null });
if (!script.startsWith('polyfills-nomodule-es5')) {
attrs.push({ name: 'defer', value: null });
}
attrs.push(
{ name: 'nomodule', value: null },
{ name: 'defer', value: null },
);
} else if (isModuleType && !isNoModuleType) {
attrs.push({ name: 'type', value: 'module' });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export function generateEntryPoints(appConfig: {
};

const entryPoints = [
'polyfills-nomodule-es5',
'runtime',
'polyfills-es5',
'polyfills',
Expand Down
4 changes: 1 addition & 3 deletions packages/angular_devkit/build_angular/src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,7 @@ export function buildWebpackBrowser(
}

// All files at this point except ES5 polyfills are module scripts
const es5Polyfills =
file.file.startsWith('polyfills-es5') ||
file.file.startsWith('polyfills-nomodule-es5');
const es5Polyfills = file.file.startsWith('polyfills-es5');
if (!es5Polyfills) {
moduleFiles.push(file);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,6 @@ export class BuildBrowserFeatures {
return !this.isFeatureSupported('es6-module');
}

/**
* Safari 10.1 and iOS Safari 10.3 supports modules,
* but does not support the `nomodule` attribute.
* While return `true`, when support for Safari 10.1 and iOS Safari 10.3
* is required and in differential loading is enabled.
*/
isNoModulePolyfillNeeded(): boolean {
if (!this.isDifferentialLoadingNeeded()) {
return false;
}

const safariBrowsers = [
'safari 10.1',
'ios_saf 10.3',
];

return this.supportedBrowsers.some(browser => safariBrowsers.includes(browser));
}

/**
* True, when a browser feature is supported partially or fully.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,69 +127,4 @@ describe('BuildBrowserFeatures', () => {
expect(buildBrowserFeatures.isFeatureSupported('es6-module')).toBe(true);
});
});

describe('isNoModulePolyfillNeeded', () => {
it('should be false for Safari 10.1 when target is ES5', () => {
host.writeMultipleFiles({
'browserslist': 'Safari 10.1',
});

const buildBrowserFeatures = new BuildBrowserFeatures(
workspaceRootSysPath,
ScriptTarget.ES5,
);
expect(buildBrowserFeatures.isNoModulePolyfillNeeded()).toBe(false);
});

it('should be false for Safari 10.1 when target is ES2015', () => {
host.writeMultipleFiles({
'browserslist': 'Safari 10.1',
});

const buildBrowserFeatures = new BuildBrowserFeatures(
workspaceRootSysPath,
ScriptTarget.ES2015,
);
expect(buildBrowserFeatures.isNoModulePolyfillNeeded()).toBe(false);
});

it('should be true for Safari 9+ when target is ES2015', () => {
host.writeMultipleFiles({
'browserslist': 'Safari >= 9',
});

const buildBrowserFeatures = new BuildBrowserFeatures(
workspaceRootSysPath,
ScriptTarget.ES2015,
);
expect(buildBrowserFeatures.isNoModulePolyfillNeeded()).toBe(true);
});

it('should be false for Safari 9+ when target is ES5', () => {
host.writeMultipleFiles({
'browserslist': 'Safari >= 9',
});

const buildBrowserFeatures = new BuildBrowserFeatures(
workspaceRootSysPath,
ScriptTarget.ES5,
);
expect(buildBrowserFeatures.isNoModulePolyfillNeeded()).toBe(false);
});

it('should be false when not supporting Safari 10.1 target is ES2015', () => {
host.writeMultipleFiles({
'browserslist': `
Edge 18
IE 9
`,
});

const buildBrowserFeatures = new BuildBrowserFeatures(
workspaceRootSysPath,
ScriptTarget.ES2015,
);
expect(buildBrowserFeatures.isNoModulePolyfillNeeded()).toBe(false);
});
});
});
59 changes: 0 additions & 59 deletions tests/legacy-cli/e2e/tests/misc/support-safari-10.1.ts

This file was deleted.

0 comments on commit 07d0879

Please sign in to comment.