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

(fix) emitCss: use deterministic css file names #30

Merged
merged 1 commit into from
Dec 17, 2017
Merged

(fix) emitCss: use deterministic css file names #30

merged 1 commit into from
Dec 17, 2017

Conversation

cristinecula
Copy link
Contributor

I have read over the thread you mentioned on my previous pull request (webpack/watchpack#25) and It struck me that there is another issue with the current implementation:

Normally multiple rebuilds shouldn't cause issues and this cause should be cached by the caller by comparing hash. I.e. the webpack CLI doesn't display stats output for equal hash.

If you run webpack multiple times over the same Svelte file, the output hash is different each time. If you take out the <style> tag, then the hash remains the same. This happens because a new temporary css file with a random name is generated each time. This could prove annoying to developers using code splitting or relying on output files including [hash] (https://medium.com/react-weekly/code-chunking-with-webpack-a-pragmatic-approach-e17e8bcc6453).

I think a safe approach would be to use ast.hash as part of the temporary file name:

	if (options.emitCss && css) {
		const tmpFile = path.join(os.tmpdir(), 'svelte-' + ast.hash + '.css');

		css += '\n/*# sourceMappingURL=' + cssMap.toUrl() + '*/';
		code = code + `\nrequire('${tmpFile}');\n`;

		writeFileSync(tmpFile, css);
		const stats = statSync(tmpFile);
		utimesSync(tmpFile, stats.atimeMs - 99999, stats.mtimeMs - 99999);
	}

The downside is that the tmp package cannot be used to generate files without the random bits (it uses mkstmp under the hood) and thus it should be removed, but it also did a cleanup of the temp files when the process exited. Without this, the /tmp folder is cleaned only after a system reboot. I'm not sure how important this is.

@Rich-Harris Rich-Harris merged commit d9955d4 into sveltejs:master Dec 17, 2017
@Rich-Harris
Copy link
Member

Thanks! Released this as 2.2.2.

Without this, the /tmp folder is cleaned only after a system reboot. I'm not sure how important this is.

I think it's fine — as long as they eventually get cleaned up, I don't see it as a big deal at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants