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

always use stats.warn instead of options.onwarn #1926

Merged
merged 2 commits into from
Dec 29, 2018
Merged
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
3 changes: 2 additions & 1 deletion src/compile/Component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export default class Component {
this.fragment = new Fragment(this, ast.html);
if (!options.customElement) this.stylesheet.reify();

this.stylesheet.warnOnUnusedSelectors(options.onwarn);
this.stylesheet.warnOnUnusedSelectors(stats);

if (!this.instance_script) {
const props = [...this.template_references];
Expand Down Expand Up @@ -229,6 +229,7 @@ export default class Component {
format,
name,
options,
this.stats,
banner,
options.sveltePath,
importedHelpers,
Expand Down
5 changes: 3 additions & 2 deletions src/compile/css/Stylesheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import removeCSSPrefix from '../../utils/removeCSSPrefix';
import Element from '../nodes/Element';
import { Node, Ast, Warning } from '../../interfaces';
import Component from '../Component';
import Stats from '../../Stats';

const isKeyframesNode = (node: Node) => removeCSSPrefix(node.name) === 'keyframes'

Expand Down Expand Up @@ -391,7 +392,7 @@ export default class Stylesheet {
});
}

warnOnUnusedSelectors(onwarn: (warning: Warning) => void) {
warnOnUnusedSelectors(stats: Stats) {
let locator;

const handler = (selector: Selector) => {
Expand All @@ -404,7 +405,7 @@ export default class Stylesheet {
const frame = getCodeFrame(this.source, start.line - 1, start.column);
const message = `Unused CSS selector`;

onwarn({
stats.warn({
code: `css-unused-selector`,
message,
frame,
Expand Down
15 changes: 3 additions & 12 deletions src/compile/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,6 @@ import renderSSR from './render-ssr/index';
import { CompileOptions, Warning, Ast } from '../interfaces';
import Component from './Component';

function normalize_options(options: CompileOptions): CompileOptions {
let normalized = assign({ generate: 'dom', dev: false }, options);
const { onwarn } = normalized;

normalized.onwarn = onwarn
? (warning: Warning) => onwarn(warning, default_onwarn)
: default_onwarn;

return normalized;
}

function default_onwarn({ start, message }: Warning) {
if (start) {
console.warn(`(${start.line}:${start.column}) – ${message}`);
Expand Down Expand Up @@ -45,10 +34,12 @@ function validate_options(options: CompileOptions, stats: Stats) {
}

export default function compile(source: string, options: CompileOptions = {}) {
options = normalize_options(options);
options = assign({ generate: 'dom', dev: false }, options);

const stats = new Stats({
onwarn: options.onwarn
? (warning: Warning) => options.onwarn(warning, default_onwarn)
: default_onwarn
});

let ast: Ast;
Expand Down
17 changes: 9 additions & 8 deletions src/compile/wrapModule.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import deindent from '../utils/deindent';
import list from '../utils/list';
import { CompileOptions, ModuleFormat, Node } from '../interfaces';
import Stats from '../Stats';

interface Dependency {
name: string;
Expand All @@ -20,6 +21,7 @@ export default function wrapModule(
format: ModuleFormat,
name: string,
options: CompileOptions,
stats: Stats,
banner: string,
sveltePath = 'svelte',
helpers: { name: string, alias: string }[],
Expand All @@ -34,7 +36,7 @@ export default function wrapModule(
}

if (format === 'cjs') return cjs(code, name, banner, sveltePath, internalPath, helpers, imports, module_exports);
if (format === 'eval') return expr(code, name, options, banner, imports);
if (format === 'eval') return expr(code, name, options, stats, banner, imports);

throw new Error(`options.format is invalid (must be ${list(Object.keys(wrappers))})`);
}
Expand Down Expand Up @@ -142,6 +144,7 @@ function expr(
code: string,
name: string,
options: CompileOptions,
stats: Stats,
banner: string,
imports: Node[]
) {
Expand Down Expand Up @@ -182,7 +185,7 @@ function expr(
return { name, statements, source: declaration.source.value };
});

const globals = getGlobals(dependencies, options);
const globals = getGlobals(dependencies, options, stats);

return deindent`
(function (${paramString(dependencies)}) { "use strict";
Expand Down Expand Up @@ -212,8 +215,8 @@ function getCompatibilityStatements(dependencies: Dependency[]) {
return statements.join('\n');
}

function getGlobals(dependencies: Dependency[], options: CompileOptions) {
const { globals, onwarn } = options;
function getGlobals(dependencies: Dependency[], options: CompileOptions, stats: Stats) {
const { globals } = options;
const globalFn = getGlobalFn(globals);

return dependencies.map(d => {
Expand All @@ -225,12 +228,10 @@ function getGlobals(dependencies: Dependency[], options: CompileOptions) {
`Could not determine name for imported module '${d.source}' – use options.globals`
);
} else {
const warning = {
stats.warn({
code: `options-missing-globals`,
message: `No name was supplied for imported module '${d.source}'. Guessing '${d.name}', but you should use options.globals`,
};

onwarn(warning);
});
}

name = d.name;
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export interface CompileOptions {

preserveComments?: boolean | false;

onwarn?: (warning: Warning) => void;
onwarn?: (warning: Warning, default_onwarn?: (warning: Warning) => void) => void;
}

export interface Visitor {
Expand Down
4 changes: 4 additions & 0 deletions test/css/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ describe('css', () => {
})
);

assert.deepEqual(dom.stats.warnings, domWarnings);

const ssr = svelte.compile(
input,
Object.assign(config, {
Expand All @@ -81,6 +83,8 @@ describe('css', () => {
})
);

assert.deepEqual(dom.stats.warnings, domWarnings);

assert.equal(dom.css.code, ssr.css.code);

assert.deepEqual(
Expand Down
41 changes: 12 additions & 29 deletions test/validator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,43 +83,26 @@ describe("validate", () => {
});

it("warns if options.name is not capitalised", () => {
const warnings = [];
svelte.compile("<div></div>", {
const { stats } = svelte.compile("<div></div>", {
name: "lowercase",
onwarn(warning) {
warnings.push({
code: warning.code,
message: warning.message,
pos: warning.pos,
start: warning.start
});
},
generate: false
});
assert.deepEqual(warnings, [
{
code: `options-lowercase-name`,
message: "options.name should be capitalised",
pos: undefined,
start: undefined
}
]);

assert.deepEqual(stats.warnings.map(w => ({
code: w.code,
message: w.message
})), [{
code: `options-lowercase-name`,
message: "options.name should be capitalised"
}]);
});

it("does not warn if options.name begins with non-alphabetic character", () => {
const warnings = [];
svelte.compile("<div></div>", {
const { stats } = svelte.compile("<div></div>", {
name: "_",
onwarn(warning) {
warnings.push({
code: warning.code,
message: warning.message,
pos: warning.pos,
start: warning.start
});
},
generate: false
});
assert.deepEqual(warnings, []);

assert.deepEqual(stats.warnings, []);
});
});