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

Update core to use new Webpack config, flarum-tsconfig, and build action #2856

Merged
merged 8 commits into from
May 12, 2021

Conversation

davwheat
Copy link
Member

Before merging this, we need to tag and publish flarum-webpack-config and update the version here.

@davwheat davwheat self-assigned this May 12, 2021
@davwheat davwheat requested a review from askvortsov1 May 12, 2021 00:05
@davwheat davwheat changed the title Update core to use new Webpack and TSConfig Update core to use new Webpack config and flarum-tsconfig May 12, 2021
js/tsconfig.json Show resolved Hide resolved
@askvortsov1 askvortsov1 added this to the 1.0 milestone May 12, 2021
@davwheat davwheat changed the title Update core to use new Webpack config and flarum-tsconfig Update core to use new Webpack config, flarum-tsconfig, and build action May 12, 2021
@davwheat davwheat force-pushed the dw/update-for-new-webpack-tsconfig branch from 35d463b to 5c1bf85 Compare May 12, 2021 21:23
Copy link
Sponsor Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Should BundleAnalyzer also be moved to webpack config?

Also, when we commit this, it should compile typings directly in this repo, right?

@davwheat
Copy link
Member Author

That might be a good idea, actually!

Yes, typings will be added to this repo under js/dist-typings.

@askvortsov1
Copy link
Sponsor Member

That might be a good idea, actually!

Yes, typings will be added to this repo under js/dist-typings.

Should we move the tooltip ones there?

@davwheat
Copy link
Member Author

No, they should be automatically added as part of the typings compilation process. (About to double check that...)

@davwheat davwheat force-pushed the dw/update-for-new-webpack-tsconfig branch from 5c1bf85 to 5337437 Compare May 12, 2021 22:18
@davwheat
Copy link
Member Author

Looks like tsc doesn't copy across .d.ts files from source "by design" 🙄

We'll need to add it the build-typings script via a cp command, or change them to .ts files.

@askvortsov1
Copy link
Sponsor Member

via a cp command

I like this one.

@davwheat
Copy link
Member Author

I was thinking the exact opposite! 😂

Changing to .ts requires no syntax modification, and automates the process compared to a manual cp that might need to be updated in the future.

@askvortsov1
Copy link
Sponsor Member

I was thinking the exact opposite! joy

I don't have a strong preference. As long as renaming them to .ts doesn't break anything I'm good with it.

@davwheat
Copy link
Member Author

I kept with .d.ts as it keeps the meaning (that they're type declarations) clear.

@askvortsov1
Copy link
Sponsor Member

Should BundleAnalyzer also be moved to webpack config?

Also, when we commit this, it should compile typings directly in this repo, right?

Are we doing this here, or in a separate PR?

This allows us to have a moving tag, like first party actions have.
@davwheat
Copy link
Member Author

Are we doing this here, or in a separate PR?

Might as well do it here while we're doing all the rest too

@davwheat
Copy link
Member Author

PR opened: flarum/flarum-webpack-config#12

@davwheat davwheat merged commit 3537f76 into master May 12, 2021
@davwheat davwheat deleted the dw/update-for-new-webpack-tsconfig branch May 12, 2021 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants