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

Review PostCSS intergration in webpack configs #724

Closed
Reinmar opened this issue Dec 15, 2017 · 10 comments
Closed

Review PostCSS intergration in webpack configs #724

Reinmar opened this issue Dec 15, 2017 · 10 comments
Assignees
Labels
type:docs This issue reports a task related to documentation (e.g. an idea for a guide). type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@Reinmar
Copy link
Member

Reinmar commented Dec 15, 2017

  1. Check whether the way we share configuration utils is easy to use.
  2. Update documentation for building CKEditor.
@Reinmar Reinmar added candidate:1.0.0 type:docs This issue reports a task related to documentation (e.g. an idea for a guide). type:task This issue reports a chore (non-production change) and other types of "todos". labels Dec 15, 2017
@Reinmar Reinmar added this to the iteration 14 milestone Dec 15, 2017
@Reinmar
Copy link
Member Author

Reinmar commented Dec 15, 2017

I also need to read through ckeditor5/build/docs/ckeditor5/1.0.0-alpha.2/framework/guides/ui/theme-customization.html which I haven't seen so far :D

@oleq
Copy link
Member

oleq commented Jan 19, 2018

A related issue #688.

@Reinmar Reinmar self-assigned this Jan 24, 2018
@Reinmar
Copy link
Member Author

Reinmar commented Mar 8, 2018

I asked @jodator to check this because it requires creating an example project in which CKEditor 5 is used as a dependency. We should check how the rules.test property can be used to apply only to CKEditor 5 stuff and whether PostCSS can be used with a different configuration to build the base application's CSS.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 8, 2018

Also, once we have some output and understanding how it works, then http://localhost/ckeditor5/build/docs/ckeditor5/1.0.0-alpha.2/framework/guides/quick-start.html can be updated with proper regexps. We may also have a section in http://localhost/ckeditor5/build/docs/ckeditor5/1.0.0-alpha.2/builds/guides/integration/advanced-setup.html about this and a link from the Quick start guide to that section.

Also, let's simplify this bit:

const { bundler } = require( '@ckeditor/ckeditor5-dev-utils' );
const { getPostCssConfig } = require( '@ckeditor/ckeditor5-dev-utils' ).styles;

By exporting all the necessary parts directly from @ckeditor/ckeditor5-dev-utils.

@jodator
Copy link
Contributor

jodator commented Mar 8, 2018

So far so good. The mentioned test was wrong as it does not allowed for nested directories. In other words only theme/file.css was true but theme/some/dir/file.css was not.

So you can use different post-css setups as you with as long as they will not interfere with CKEdfitor 5 ones.

ie.

{
	test: /ckeditor5-[^/]+\/theme\/[\w-/]+\.css$/,
	use: [
		'style-loader',
		{
			loader: 'postcss-loader', options: { /**/ }
		},
	]
},
{
	test: /app.css$/,
	// test /.css$/, // will not work as it will try to parse CKEdtior5 files again
	use: [
		'style-loader',
		{
			loader: 'postcss-loader', options: { parser: 'sugarss' }
		},
	]
},

@jodator
Copy link
Contributor

jodator commented Mar 9, 2018

Also, let's simplify this bit:

const { bundler } = require( '@ckeditor/ckeditor5-dev-utils' );
const { getPostCssConfig } = require( '@ckeditor/ckeditor5-dev-utils' ).styles;

By exporting all the necessary parts directly from @ckeditor/ckeditor5-dev-utils.

I can see that @ckeditor/ckeditor5-dev-utils. exports many things that are not used. For the styles namespace I can see only getPostCssConfig is being used. The simples solution would be to modify exports:

const styles = require( './styles/index' );

module.exports = {
	git: require( './git' ),
	logger: require( './logger' ),
	tools: require( './tools' ),
	stream: require( './stream' ),
	workspace: require( './workspace' ),
	translations: require( './translations/index' ),
	bundler: require( './bundler/index' ),
	styles,
	getPostCssConfig: styles.getPostCssConfig
};

But it looks weird. I'm OK with this as the mostly used options are bundler, getPostCssConfig and sometimes other:
selection_012

@Reinmar
Copy link
Member Author

Reinmar commented Mar 9, 2018

So, maybe we could only change the usage of this pkg in build pkgs. To import { bunlder, styles } and then use styles.getPostCssConfig()?

@jodator
Copy link
Contributor

jodator commented Mar 9, 2018

👍 I think that it would be a bit cleaner. I'll prepare a PR do dev-utils,

@Reinmar
Copy link
Member Author

Reinmar commented Mar 9, 2018

Do we need a PR there? Unless I miss something, it seems that we just need to change ckeditor5-build-*s?

@jodator
Copy link
Contributor

jodator commented Mar 9, 2018

Sorry I've ment PR to the ckeditor5-* repos.

pomek added a commit to ckeditor/ckeditor5-build-balloon that referenced this issue Mar 12, 2018
Other: Unify the `getPostCssConfig()` import from `@ckeditor/ckeditor5-dev-utils` package. See ckeditor/ckeditor5#724.
pomek added a commit to ckeditor/ckeditor5-build-classic that referenced this issue Mar 12, 2018
Other: Unify the `getPostCssConfig()` import from `@ckeditor/ckeditor5-dev-utils` package. See ckeditor/ckeditor5#724.
pomek added a commit to ckeditor/ckeditor5-build-inline that referenced this issue Mar 12, 2018
Other: Unify the `getPostCssConfig()` import from `@ckeditor/ckeditor5-dev-utils` package. See ckeditor/ckeditor5#724.
JDinABox pushed a commit to JDinABox/ckeditor5-build-markdown that referenced this issue Sep 6, 2021
Other: Unify the `getPostCssConfig()` import from `@ckeditor/ckeditor5-dev-utils` package. See ckeditor/ckeditor5#724.
rivernews pushed a commit to rivernews/ckeditor5-build-balloon-2022 that referenced this issue Dec 31, 2021
Other: Unify the `getPostCssConfig()` import from `@ckeditor/ckeditor5-dev-utils` package. See ckeditor/ckeditor5#724.
fl01337 pushed a commit to fl01337/ckeditor-5-build-inline that referenced this issue May 19, 2022
Other: Unify the `getPostCssConfig()` import from `@ckeditor/ckeditor5-dev-utils` package. See ckeditor/ckeditor5#724.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:docs This issue reports a task related to documentation (e.g. an idea for a guide). type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

3 participants