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

Convert plugin to loader #4

Closed
fregante opened this issue Mar 31, 2020 · 10 comments
Closed

Convert plugin to loader #4

fregante opened this issue Mar 31, 2020 · 10 comments
Labels
breaking change impact: high unblocks new usecases, substantial improvement to existing feature, fixes a major bug status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response type: feature or enhancement An issue or pull request that adds a feature or enhances a current one type: question or discussion Issue discussing or asking a question

Comments

@fregante
Copy link

Instead of having the manifest inside webpack.config.js, webpack should just read the input json file and output it transformed. Something like:

const targetBrowser = process.env.TARGET_BROWSER;

module.exports = {
	// ...

	entry: {
		manifest: './source/manifest.json',
		// ...
	},
	output: {
		path: path.join(__dirname, "extension", targetBrowser),
		filename: "[name].[ext]"
	},
	module: {
		rules: [
			{
				test: /manifest\.json$/,
				use: 'wext-manifest-loader', // automatically uses the `TARGET_BROWSER` env
				exclude: /node_modules/
			}
		]
	},
};
@abhijithvijayan
Copy link
Owner

abhijithvijayan commented Mar 31, 2020

oh cool. I will work on it.

This will cause breaking changes but still it seems to be worth it.

will have to deprecate this as well.

@abhijithvijayan abhijithvijayan added breaking change impact: high unblocks new usecases, substantial improvement to existing feature, fixes a major bug type: feature or enhancement An issue or pull request that adds a feature or enhances a current one labels Mar 31, 2020
@abhijithvijayan
Copy link
Owner

abhijithvijayan commented Mar 31, 2020

This is infact an awesome idea that not only improves its function but also adds the manifest.json reloadabilty(which it lacked in the starter project)

@abhijithvijayan
Copy link
Owner

@fregante what do you suggest as to read the version field for the extension?

Currently it uses package.json file version.

Or is it better to just add version property to manifest.json?

@fregante
Copy link
Author

@abhijithvijayan abhijithvijayan added the status: WIP Work in progress ... inputs welcome! ❤️ label Apr 1, 2020
@abhijithvijayan
Copy link
Owner

abhijithvijayan commented Apr 1, 2020

@fregante Caveats that I found so far

Loader now works with TARGET_BROWSER env passed with chrome/firefox/opera/edge and type: "javascript/auto"

I builds manifest.json to root of build directory along with a js bundle

Unless having a plugin in webpack, I don't think the emitting of manifest.js can be avoided.

I can write a plugin to do this. Do you have any better suggestions?

@fregante
Copy link
Author

fregante commented Apr 1, 2020

Yeah this might not be as easy as I thought, but I think this loader can generate files with the right extension: https://github.com/peerigon/extract-loader/blob/master/README.md

@abhijithvijayan

This comment has been minimized.

@abhijithvijayan

This comment has been minimized.

@abhijithvijayan
Copy link
Owner

abhijithvijayan commented Apr 2, 2020

@fregante The loader emits correctly like you asked for. I have build a plugin to remove the manifest from compilation(to a js bundle).

Here is the plugin: https://github.com/abhijithvijayan/wext-manifest-webpack-plugin

The loader is released to: https://www.npmjs.com/package/wext-manifest-loader

The only additional optional feature I will add to this loader will be the ability to take version from package.json as an option to update manifest's version field(This will be optional)

I will deprecate the wext-manifest package once I set up a migration guide for existing users.

Checkout the readme example for the usage

I hope you can use these in https://github.com/sindresorhus/refined-github once you verify if everything works like you asked for.

@abhijithvijayan abhijithvijayan pinned this issue Apr 2, 2020
@abhijithvijayan abhijithvijayan added status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response type: question or discussion Issue discussing or asking a question and removed status: WIP Work in progress ... inputs welcome! ❤️ labels Apr 2, 2020
@abhijithvijayan
Copy link
Owner

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change impact: high unblocks new usecases, substantial improvement to existing feature, fixes a major bug status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response type: feature or enhancement An issue or pull request that adds a feature or enhances a current one type: question or discussion Issue discussing or asking a question
Projects
None yet
Development

No branches or pull requests

2 participants