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

Generating *.meta.js during production build #6

Merged
merged 1 commit into from
Nov 29, 2024
Merged

Conversation

Giglium
Copy link
Contributor

@Giglium Giglium commented Oct 28, 2024

This draft PR want to add the feature of the automatic generation of the *.meta.js files. I'm looking for a feedback or a confirmation if this is the appropriate direction.

I'm don't really like the idea to user the replace function to generate the meta filenames, but is the most efficient way that I found since I prefer to avoid to generate twice the metaBlock. Any other ideas ?

In your opinion, we should introduce a flag to enable or disable the generation of these meta files?

Thank you for your feedback!

@Giglium Giglium marked this pull request as draft October 28, 2024 07:46
@Giglium
Copy link
Contributor Author

Giglium commented Nov 22, 2024

Hi @guansss,
Do you had a chance to review this PR?

Thanks for your time!

@guansss
Copy link
Owner

guansss commented Nov 25, 2024

Hi, sorry for the late response! And thank you for this PR, this is a great feature to have!

I wonder why the meta file goes to chunk.auxiliaryFiles? I haven't used it before so I'm not sure what differences it will make.

Using replace to generate the meta filename is definitely okay since the js output's filename is fixed as *.user.js. But there is a pitfall in .replace("user", "meta"), where something like "user-blocker.user.js" will be converted to "meta-blocker.user.js". It should probably be .replace(/\.user\.js$/, ".meta.js").

Having a flag would be nice. I imagine something like options.meta.file or options.meta.outFile?

@Giglium
Copy link
Contributor Author

Giglium commented Nov 25, 2024

Thanks for the review! Certainly, regex is the most robust way to perform the replace avoiding problems.

For the auxiliaryFiles I have used this guide as a reference: webpack/webpack#11425 (comment)
I understand that we should use them for any file generated during the build that is not a .js or .css. For this reason, I used them.

For the flag, I was thinking something like options.meta.generate with true as the default value. The options.meta.file or options.meta.outFile let me think of having the option to specify the meta filename or output file but I won't provide this possibility...or we should?

In the next few days, I will update the PR with the modifications.

@Giglium Giglium marked this pull request as ready for review November 26, 2024 07:44
@Giglium Giglium changed the title [Draft] Generating *.meta.js during production build Generating *.meta.js during production build Nov 26, 2024
@guansss
Copy link
Owner

guansss commented Nov 29, 2024

Thanks for the update!

In my opinion, the word "generate" fits well, but as an option, it may be a bit too broad and could represent the entire process of converting the original meta.js to the meta comment block (both in *.user.js and *.meta.js). I think at least the word "file" should be included to indicate that we are going to generate an extra file. Maybe options.meta.generateFile?

src/node/MonkeyPlugin.ts Outdated Show resolved Hide resolved
@Giglium
Copy link
Contributor Author

Giglium commented Nov 29, 2024

Changed to options.meta.generateFile it's good! I've also squashed all the commit into one!

Copy link
Owner

@guansss guansss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@guansss guansss merged commit 88df2d4 into guansss:main Nov 29, 2024
1 check failed
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