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

@wordpress/scripts: sourcemap missing in builds #44278

Open
kkmuffme opened this issue Sep 19, 2022 · 8 comments · Fixed by #46812 · May be fixed by #45251
Open

@wordpress/scripts: sourcemap missing in builds #44278

kkmuffme opened this issue Sep 19, 2022 · 8 comments · Fixed by #46812 · May be fixed by #45251
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.

Comments

@kkmuffme
Copy link
Contributor

Description

@wordpress/scripts script wp-scripts build doesn't generate a sourcemap (only wp-scripts start will)

This means all production errors cannot be investigated or fixed, as all reported info (e.g. in Sentry) is useless.

It seems this was changed already #33718 but this doesn't affect @wordpress/scripts? (sorry JS isn't my area of expertise)
(kind of related #23960, but I think that issue is already fixed by the above PR)

@gziolo could you please quickly change it or point me towards where I can change and PR this? Thanks!

Step-by-step reproduction instructions

run "npm run build", it won't generate a .map

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@noahtallen
Copy link
Member

noahtallen commented Sep 19, 2022

It looks like you can use WP_DEVTOOL for development builds here:

config.devtool = process.env.WP_DEVTOOL || 'source-map';

I think a reasonable change would be to allow WP_DEVTOOL to control the webpack devtool property for production builds as well, and the default could just be to not generate sourcemaps. People who need them for production builds could then enable them by adding WP_DEVTOOL. I think you could add that somewhere near here:

entry: getWebpackEntryPoints,

@gziolo gziolo added [Type] Enhancement A suggestion for improvement. Needs Dev Ready for, and needs developer efforts [Tool] WP Scripts /packages/scripts labels Sep 21, 2022
@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Sep 21, 2022
@gziolo
Copy link
Member

gziolo commented Sep 21, 2022

It's false by default according to https://webpack.js.org/configuration/devtool/#devtool. So what @noahtallen said might work. I'm not quite sure what role the source map loader plays in that:

config.module.rules.unshift( {
test: /\.(j|t)sx?$/,
exclude: [ /node_modules/ ],
use: require.resolve( 'source-map-loader' ),
enforce: 'pre',
} );

So maybe we should include that when process.env.WP_DEVTOOL is set to any correct value or it defaults to 'source-map' when in development mode.

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 24, 2022
@Chrico
Copy link
Contributor

Chrico commented Dec 28, 2022

Heyho,

I just wanted to jump in to ask about the current state of this ticket. What I can see is, that yarn build does only produce a minified version, while yarn build --mode=none produces an unminified version and yarn build --mode=development a version which uses eval(). But all 3 are without sourcemap.

See modes here: https://webpack.js.org/configuration/mode/

Overall the yarn start is not usable when you want to build assets via Github Actions, because the process does not stop itself after building the assets. So for me there are only one way to go:

Add to the yarn build the --webpack--devtool -arg and allow building the source map as well and extend the packages/scripts/config/webpack.config.js to set not only in !isProduction. It should be set when the flag is not empty.

Available values are defined here: https://webpack.js.org/configuration/devtool/

This soltuion does not introduce any breaking change, but allows to build a source map when you decide that you need it. So the default behavior does not change.

command expected result
yarn build behaves like the current one, minified and no sourcemap
yarn build --webpack-devtool=sourcemap like above but with sourcemap
yarn build --mode=none --webpack-devtool=sourcemap unminified and sourcemap

@noahtallen
Copy link
Member

I quickly created #46812, as an alternative approach, using the existing WP_DEVTOOL env variable rather than adding a flag. So WP_DEVTOOL=source-map yarn build would work.

I'm also happy with the idea of creating sourcemaps by default as in #45251. We just need some more core people to sign off

kkmuffme added a commit to kkmuffme/gutenberg that referenced this issue Dec 29, 2022
There is no reason production builds should not include sourcemaps by default - without sourcemaps, debugging the error or - for users - reporting errors is not possible, making the website and the web in general less accessible

* analogous to WordPress#33718
* Fix: WordPress#44278
* Fix: WordPress#41551
@Chrico
Copy link
Contributor

Chrico commented Jan 4, 2023

Any update here? Will this be part of the next release of @wordpress/scripts ?

@gziolo
Copy link
Member

gziolo commented Feb 2, 2023

Any update here? Will this be part of the next release of @wordpress/scripts ?

I have just landed #46812 to resolve this issue. There is still #45251 in works, that proposes creating souremaps by default which we might land later.

As for the release date, we had npm publishing yesterday, so we need to wait another 2 weeks because the process is tied to the Gutenberg plugin release cycle.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label May 17, 2023
@acketon
Copy link

acketon commented Mar 15, 2024

It looks like the pr #46812 was merged and I was able to get the source map for my JS file to be created during the build script but not the source map for the SASS files.

In our environment we find it very useful to publish source maps for our compiled SASS / CSS for testing/debugging in production and being able to apply a quick css hotfix. I've been trying for a few hours to find a way to do this but it seems the webpack.config.js that wp-scripts provides only generates the sourceMap for CSS files if we are not in the production mode. See these lines here, here, and here

I've been attempting to use webpack-merge to change that value for each of the CSS loaders but yikes is that tricky. This setting in wp-scripts seems quite opinionated to me and it would be ideal if there was an easier way to modify the setting so we can generate source maps even with our production builds.

We're trying to move from an old grunt based build process to wp-scripts and mirror our previous configuration but some of these strong defaults are difficult to figure out how to modify it.

@kkmuffme
Copy link
Contributor Author

kkmuffme commented Mar 16, 2024

@gziolo thanks, but can you please reopen the issue? While there's an option to include sourcemaps, this should become the default behavior to keep WP an open and accessible ecosystem.
If you use various gutenberg plugins, you basically have to manually re-build every plugin to get sourcemaps, otherwise your error logging tools (e.g. sentry) report gibberish.

Open to discussion, also on the PR since nobody replied to the arguments in the end #45251 (comment)

@gziolo gziolo reopened this Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Tool] WP Scripts /packages/scripts [Type] Enhancement A suggestion for improvement.
Projects
None yet
6 participants