microbundle icon indicating copy to clipboard operation
microbundle copied to clipboard

Add `x_google_ignoreList` (Ignore-listing code) support to sourcemaps

Open 0xdevalias opened this issue 1 year ago • 9 comments

Introduce the x_google_ignoreList extension in the sourcemaps generated by this project. This will facilitate a more streamlined debugging experience in Chrome (and other supporting browsers) by automatically filtering out framework and dependency code.

Benefit

Implementing x_google_ignoreList in the sourcemaps will align this project with modern web development practices, offering a more focused and efficient debugging experience for developers using Chrome DevTools. This change will particularly benefit those who regularly engage in debugging complex applications with numerous dependencies.

Background Context

  • https://developer.chrome.com/blog/devtools-modern-web-debugging/#just-my-code
    • The ignore-listing is enabled by populating an extra x_google_ignoreList field in the source map that Angular generates when compiling the project. The Chrome DevTools team encourages other framework authors to implement a similar change. Refer to the Case Study: Better Angular Debugging with DevTools post for more details.

  • https://developer.chrome.com/blog/devtools-better-angular-debugging/#ignore-listing-code
    • Ignore-listing code When debugging applications using Chrome DevTools, authors generally only want to see just their code, not that of the framework underneath or some dependency tucked away in the node_modules folder.

      To achieve this, the DevTools team has introduced an extension to source maps, called x_google_ignoreList. This extension is used to identify third-party sources such as framework code or bundler-generated code. When a framework uses this extension, authors now automatically avoid code that they don't want to see or step through without having to manually configure this beforehand.

      The x_google_ignoreList source map extension is supported by Chrome DevTools starting with Chrome 106.

  • https://developer.chrome.com/articles/x-google-ignore-list/
    • The ignoreList source map extension

      Improve debugging experience in Chrome DevTools with the ignoreList source map extension.

  • https://developer.chrome.com/blog/new-in-devtools-120/#ignore-list-spec
    • The source maps specification has adopted the ignoreList field instead of x_google_ignoreList and DevTools now supports the new name with a fallback for the old one. Frameworks and bundlers can now use the new field name.

Originally posted by @0xdevalias in https://github.com/facebook/react/issues/27774#issuecomment-1836969520

  • https://tc39.es/source-map-spec/#ignorelist

See Also

  • https://github.com/facebook/react/issues/27774
  • https://github.com/markerikson/react-prod-sourcemaps/issues/19
  • https://github.com/vercel/next.js/issues/41370
    • https://github.com/vercel/next.js/discussions/41371
  • https://github.com/vuejs/rfcs/discussions/607
  • https://github.com/sveltejs/svelte/issues/9740
  • https://github.com/angular/angular-cli/pull/23545
  • https://nuxt.com/blog/v3-3#better-logging-in-browser-devtools
    • https://github.com/nuxt/nuxt/pull/19243
  • https://vitejs.dev/config/server-options.html#server-sourcemapignorelist
    • https://github.com/vitejs/vite/pull/12174
    • https://github.com/vitejs/vite/pull/12633
    • https://github.com/vitejs/vite/pull/12694
  • https://rollupjs.org/configuration-options/#output-sourcemapignorelist
    • https://github.com/rollup/rollup/issues/4847
    • https://github.com/rollup/rollup/pull/4846
    • https://github.com/rollup/rollup/pull/4848
    • https://github.com/rollup/rollup/pull/4926
  • https://github.com/Rich-Harris/magic-string/issues/241
    • https://github.com/Rich-Harris/magic-string/pull/240
    • https://github.com/Rich-Harris/magic-string/pull/243

0xdevalias avatar Dec 02 '23 01:12 0xdevalias

Microbundle is a tool for building libraries, not apps. So again, wrong layer as this should be implemented in the tools that consume Microbundle's output.

Also, I don't think opening these copy/pasted issues everywhere is going to get you the desired reaction.

rschristian avatar Dec 02 '23 01:12 rschristian

Microbundle is a tool for building libraries, not apps. So again, wrong layer as this should be implemented in the tools that consume Microbundle's output.

@rschristian This context from the other thread that explains why I opened it here:

Fair enough. Well you may or may not find this issue relevant there then, since it's a bundler (though I'm not really sure how a thing like this that should be configurable would be exposed through a 'no config' tool):

  • https://github.com/developit/microbundle/issues/1066

Originally posted by @0xdevalias in https://github.com/preactjs/preact/issues/4225#issuecomment-1836982571


Also, I don't think opening these copy/pasted issues everywhere is going to get you the desired reaction.

@rschristian I'm not sure what you're assuming my 'desired reaction' is here. I'm trying to get a useful feature implemented across the major web frameworks. It's copy pasted because there is no point individually re-writing the same context and details every time since it will be exactly the same. They are cross-linked so that it's easier to find the requests, any relevant comments/context, and implementation specifics between the frameworks to reduce the effort of having to 'start from scratch' for each.

0xdevalias avatar Dec 02 '23 01:12 0xdevalias

This context from the other thread that explains why I opened it here:

Fair enough. Well you may or may not find this issue relevant there then, since it's a bundler

Per the tag line, this is a tool for modules, not apps.

I get why you opened it but, like Preact's repo, it doesn't fit here. Everything Microbundle outputs is meant for use in other tools really, tools which can and should control this setting themselves.


I'm not sure what you're assuming my 'desired reaction' is here.

Your desired reaction would be a positive one, I'd imagine.

The problem is that this comes off as incredibly spammy, especially as it's mistargeted. Copy/pasting issues on a bunch of repos just isn't a good way to go about things. I fully understand you mean well but this isn't the way to do it.

rschristian avatar Dec 02 '23 01:12 rschristian

Copy/pasting issues on a bunch of repos just isn't a good way to go about things. I fully understand you mean well but this isn't the way to do it.

Genuine question: Ignoring the 'mistargeted' aspect (assuming that's true and I've just misunderstood); if someone has a desire to have a useful ecosystem feature considered across a number of projects in a related category (eg. web frameworks/'UI libraries'/etc; what would you consider to be 'the way to do it'? As this seemed like the most useful and direct method I could think of for doing so.

0xdevalias avatar Dec 02 '23 03:12 0xdevalias

I still think this should be done on the bundler/app/user side of things, rather than in libraries, but if you want to contribute a fix, go for it.


if someone has a desire to have a useful ecosystem feature considered across a number of projects in a related category (eg. web frameworks/'UI libraries'/etc; what would you consider to be 'the way to do it'?

Frankly, you shouldn't.

If you want to see a feature land in a project you actually use, then sure, go ahead and open an issue, perhaps with a few "prior art" links. But copy/pasting the same request across multiple projects just because you read about a new feature can come off as quite demanding on OSS maintainers. Imagine if everyone was doing this.

But if did insist on doing it, you should be shortly opening a PR yourself alongside that issue request (an example of this was when some NPM people went around correcting popular packages' "repository" fields as many were invalid or using perhaps ill-spec'd versions). Asking for the entire ecosystem to change, without investing your own time in landing it, is simply not healthy in OSS.

rschristian avatar Dec 02 '23 22:12 rschristian

I still think this should be done on the bundler/app/user side of things, rather than in libraries, but if you want to contribute a fix, go for it.

@rschristian I'll have to dig deeper into the code/etc here before I could answer this one way or another myself, but in case that doesn't turn up a good answer, maybe I can pre-empt getting stuck and ask ahead of time.

Given microbundle is a 'Zero-configuration bundler', my assumption was that was literal and there was no way for a downstream user to configure it; but quickly skimming the README it seems there may be a few options available. Do you have a preference for how I implement this? Should it be a default on, and/or added as a new configurable option, etc?


If you want to see a feature land in a project you actually use, then sure, go ahead and open an issue, perhaps with a few "prior art" links. But copy/pasting the same request across multiple projects just because you read about a new feature can come off as quite demanding on OSS maintainers. Imagine if everyone was doing this.

@rschristian (Mini essay here, didn't intend it to be so long; definitely no pressure/expectation to read it all/reply in detail)

Somewhat off topic 'mini essay' response about my process

I understand your view here, though it's making some assumptions that aren't true in this case. It's not just that I 'read about a new feature' and wanted to spam it across a bunch of projects, but because that new feature would be specifically useful in my downstream workflows (eg. web app analysis), and that it being adopted across the top web frameworks/UI libraries would mean that majority of apps I come across would then support it by default.

As far as it coming off as demanding, that's definitely not my intention. I don't open issues to 'demand'; I raise a thing (bug, feature, etc) that I think would benefit the project, as an invitation to a conversation/collaboration around the thing being raised. Ultimately if it's not something a project is willing to implement/accept, that's fair enough, it's not my place to demand it (though as you no doubt inferred from our conversation on the other issue, my neurodiverse brain wants to understand the "why can't it" if the basis of a 'no' is simply more than "I don't want that in my project"; as I've found that often people can quickly jump to a conclusion that doesn't always hold true under further exploration)

Then, depending on what the thing being raised is, my time/energy (and whether my ADHD brain remembers to), I will then often try and dig into some of the deeper specifics as relevant to that specific project, and add further context on that issue for how I think it could be implemented/etc; though as I am often not deeply familiar with the projects, it can take more time/effort to figure out those specifics, and I like to have somewhere created to 'put those learnings' when I do (thus the issue). Also sometimes a maintainer jumps in before I get a chance to look at those deeper things and has ideas/pointers for where to look to implement it (or if it's something they won't accept regardless, and thus not worth me looking deeper into it).

Then finally, based on that analysis/context, either I or another contributor should be able to more quickly/easily make the specific changes required to implement/fix the thing.

I guess I sort of see it as a 'staged process', with each stage aiming to contribute some value, even if I don't manage to keep my brain/memory focussed on it long enough to see it through to the fully completed solution.

But if did insist on doing it, you should be shortly opening a PR yourself alongside that issue request

@rschristian That's totally fair. As I sort of inferred in my 'mini-essay' above, I often don't want to jump straight to a PR before there has been at least some level of "yes, sounds good" from a project; as it's not worth sinking my time/energy into implementing it if the project will just turn around and be like "no, we don't want that".

Asking for the entire ecosystem to change, without investing your own time in landing it, is simply not healthy in OSS.

@rschristian nods agreed.

0xdevalias avatar Dec 02 '23 23:12 0xdevalias

Do you have a preference for how I implement this? Should it be a default on, and/or added as a new configurable option, etc?

Sorry, meant to say that enabling it is fine, no need for config.

I don't love that package maintainers will need to tell their consumers that they'll have to alter their browser's settings if they want the old behavior (as this seems to be enabled by default in Chrome), but we have too many flags here as-is, IMO.

I don't open issues to 'demand'; I raise a thing (bug, feature, etc) that I think would benefit the project, as an invitation to a conversation/collaboration around the thing being raised.

I often don't want to jump straight to a PR before there has been at least some level of "yes, sounds good" from a project; as it's not worth sinking my time/energy

I get that, but unfortunately collaboration like this demands a time investment from someone: either the (potential) contributor doing work up front or the maintainers fielding questions. I know it seems like a quick thing but many of us have another 50+ repos we're fielding questions in (and even more that we're neglecting) on top of work, personal life, etc.

But again, that's just me. Opening issues for discussion/one-offs are a-okay, but if you're trying to standardize an entire ecosystem, personally, it comes off a lot more positive if you're making some initial investment to make that happen. A hacky, post-build injection in Preact's source maps (for this issue) would've been a fine starting point, for example.

rschristian avatar Dec 03 '23 01:12 rschristian

enabling it is fine, no need for config

@rschristian Ok, thanks for clarifying :)

I don't love that package maintainers will need to tell their consumers that they'll have to alter their browser's settings if they want the old behavior (as this seems to be enabled by default in Chrome)


@rschristian nods, understandable. Though it sounds like in many cases a new default coming in Chrome 120 would have a similar impact on a majority of bundled code anyway:

  • https://developer.chrome.com/blog/new-in-devtools-120/#default-regex
    • This version enables the default regular expression as a custom exclusion rule in Settings. Settings > Ignore List. To help you focus on only your code, the Debugger will now skip scripts from /node_modules/ and /bower_components/ by default. You can disable this rule in settings at any time.

And I guess for a lot of 'end-users' of libraries, in a majority of cases they aren't ever going to need to switch back to the old behaviour, as the internals of a lib aren't often super relevant to debugging web app code.

I'd expect it to theoretically be more of a potential concern to library maintainers themselves, but then they're probably not developing on the bundled/minimised version of their lib anyway, so this shouldn't impact them there either.


I get that, but unfortunately collaboration like this demands a time investment from someone: either the (potential) contributor doing work up front or the maintainers fielding questions. I know it seems like a quick thing but many of us have another 50+ repos we're fielding questions in (and even more that we're neglecting) on top of work, personal life, etc.

@rschristian nods that's totally fair, and I can understand and relate to that feel.

Opening issues for discussion/one-offs are a-okay, but if you're trying to standardize an entire ecosystem, personally, it comes off a lot more positive if you're making some initial investment to make that happen. A hacky, post-build injection in Preact's source maps (for this issue) would've been a fine starting point, for example.

@rschristian nods fair enough, I can definitely take that feedback on board :)

0xdevalias avatar Dec 04 '23 03:12 0xdevalias

Tracing the right place to make these changes
  • https://github.com/developit/microbundle#-installation--setup--
    • Try it out by running npm run build.

  • https://github.com/developit/microbundle/blob/master/package.json#L22-L24
    • "build": "npm run -s build:babel && npm run -s build:self",
      "build:babel": "babel-node src/cli.js --target=node --format cjs src/{cli,index}.js",
      "build:self": "node dist/cli.js --target=node --format cjs src/{cli,index}.js",
      
  • https://github.com/developit/microbundle/blob/master/src/cli.js#L8-L9
    • import microbundle from './index';
  • https://github.com/developit/microbundle/blob/master/src/index.js#L50
    • export default async function microbundle(inputOptions) {
        // ..snip..
        let steps = [];
          for (let i = 0; i < options.entries.length; i++) {
            for (let j = 0; j < formats.length; j++) {
              steps.push(
                createConfig(options, options.entries[i], formats[j], j === 0),
              );
            }
          }
        // ..snip..
        let out = await series(
            steps.map(config => async () => {
              const { inputOptions, outputOptions } = config;
      		if (inputOptions.cache !== false) {
      			inputOptions.cache = cache;
      		}
              let bundle = await rollup(inputOptions);
              cache = bundle;
              await bundle.write(outputOptions);
              return await config._sizeInfo;
            }),
          );
        // ..snip..
      
  • https://github.com/developit/microbundle/blob/master/src/index.js#L326
    • function createConfig(options, entry, format, writeMeta) {
      
    • https://github.com/developit/microbundle/blob/master/src/index.js#L451-L702
      • This seems to be the actual config that contains inputOptions passed to rollup and outputOptions passed to bundle.write

The change to be made is in rollup's output config:

  • https://rollupjs.org/configuration-options/#output-sourcemapignorelist
    • output.sourcemapIgnoreList Type: boolean | (relativeSourcePath: string, sourcemapPath: string) => boolean A predicate to decide whether or not to ignore-list source files in a sourcemap, used to populate the x_google_ignoreList source map extension. When you don't specify this option explicitly, by default it will put all files with node_modules in their path on the ignore list. You can specify false here to turn off the ignore-listing completely.

Based on the description of that rollup config option, since microbundle is designed for libraries, I believe it should be fine to just pass true here, and ignore-list all of the library files.

Based on the above exploration of microbundle's code, it looks like the outputOptions config passed to rollup are defined here:

  • https://github.com/developit/microbundle/blob/master/src/index.js#L684-L701

So the entire change to implement this is probably just:

  // ..snip..

  outputOptions: {
    // ..snip..
    sourcemap: options.sourcemap,
+   sourcemapIgnoreList: true,
    // ..snip..
  },

  // ..snip..

After making that, I was getting some warnings while running npm test:

console.warn
    Unknown output options: sourcemapIgnoreList. Allowed options: amd, assetFileNames, banner, chunkFileNames, compact, dir, dynamicImportFunction, entryFileNames, esModule, exports, extend, externalLiveBindings, file, footer, format, freeze, globals, hoistTransitiveImports, indent, inlineDynamicImports, interop, intro, manualChunks, minifyInternalExports, name, namespaceToStringTag, noConflict, outro, paths, plugins, preferConst, preserveModules, preserveModulesRoot, sourcemap, sourcemapExcludeSources, sourcemapFile, sourcemapPathTransform, strict, systemNullSetters

output.sourcemapIgnoreList was added to rollup in 3.16.0 (2023-02-17):

  • https://github.com/rollup/rollup/blob/master/CHANGELOG.md#3160
    • Support output.sourcemapIgnoreList option to mark file sources as ignored in the x_google_ignoreList attribute of the resulting sourcemap (rollup/rollup#4848)

microbundle is using a super outdated version of rollup (2.35.1) from 2020-12-14:

  • https://github.com/developit/microbundle/blob/c76c41f8317611f7592d8e44c569e8083076f25f/package.json#L102

So before we can implement this change, we'll need to upgrade to a modern version of rollup.


I'll work on getting that change implemented and hopefully put up a PR for it shortly.

Edit: Maybe not so shortly, as it looks like microbundle's version of rollup is super outdated (using 2.35.1, and this flag isn't supported till 3.16.0. Opened a new issue to track this:

  • https://github.com/developit/microbundle/issues/1067

0xdevalias avatar Dec 04 '23 04:12 0xdevalias