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

Meta: Clean up webpack config file #4201

Merged
merged 10 commits into from Apr 6, 2021
Merged

Meta: Clean up webpack config file #4201

merged 10 commits into from Apr 6, 2021

Conversation

fregante
Copy link
Member

@fregante fregante commented Apr 5, 2021

This extracts code and checks unrelated to the build to its own file.

Ideally, separately, the actual checks would be completely separate from webpack. They'd require this same file, but they wouldn't throw errors.

@fregante fregante added the meta Related to Refined GitHub itself label Apr 5, 2021
@yakov116
Copy link
Member

yakov116 commented Apr 5, 2021

@fregante can you update webpack too? I tried updating and I was unable to build

package.json Outdated Show resolved Hide resolved
@fregante
Copy link
Member Author

fregante commented Apr 5, 2021

It was webpack-contrib/css-loader#1284

xo --fix failing again
build/readme-parser.ts Outdated Show resolved Hide resolved
Co-authored-by: yakov116 <16872793+yakov116@users.noreply.github.com>
webpack.config.ts Outdated Show resolved Hide resolved
fregante and others added 3 commits April 5, 2021 21:29
Co-authored-by: yakov116 <16872793+yakov116@users.noreply.github.com>
@@ -0,0 +1,65 @@
/// <reference types="../source/globals" />
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated question. Why is this needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't ask me!

I think I tried moving globals to the root and use it there but it didn't work…

@yakov116
Copy link
Member

yakov116 commented Apr 5, 2021

Does build need to be added here?

https://github.com/sindresorhus/refined-github/blob/a34191037be2ce5ad44ca6cc7015f71c8c8a7b19/tsconfig.json#L11-L14

If yes, once your at it "octicon-svg-loader.ts" can be removed

Copy link
Member

@yakov116 yakov116 left a comment

Choose a reason for hiding this comment

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

Untested but looks good.

Want to regenerate the lock?

@fregante
Copy link
Member Author

fregante commented Apr 5, 2021

It’s ok for now since only select dev dependencies have been updated, we don’t have to do it every time.

@yakov116
Copy link
Member

yakov116 commented Apr 5, 2021

It’s ok for now since only select dev dependencies have been updated, we don’t have to do it every time.

I got burned from this so many times. I wish I would know the reason, but since we switched to npm 15 every time I do an update if I don't delete and start over, 💥.

Probably has to do with my machine. 🤷

@fregante
Copy link
Member Author

fregante commented Apr 6, 2021

Yeah it does happen but I supposed that if it works on CI it works for everyone. Usually they're peer dependency errors where babel stuff got updated partially and it's not fully compatible across versions.

@fregante
Copy link
Member Author

fregante commented Apr 6, 2021

Tested both build and watch

@fregante fregante changed the title Meta: Cleanup webpack config file Meta: Clean up webpack config file Apr 6, 2021
@fregante fregante merged commit 0816b75 into main Apr 6, 2021
@fregante fregante deleted the webpack-clean branch April 6, 2021 02:10
@yakov116
Copy link
Member

yakov116 commented Apr 6, 2021

Me too :)

Just a heads up I really learned from this PR. How you put the code together so simply and understandable.

I look at the code and I think to myself "ya maybe one day..."

Side Note: When do you sleep?!

@fregante
Copy link
Member Author

When do you sleep?!

I recently moved across 12 timezones. I might have normalized my sleep schedule for now 😂

@yakov116
Copy link
Member

I was wondering!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Related to Refined GitHub itself
Development

Successfully merging this pull request may close these issues.

None yet

2 participants