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

Get sourcemaps working #4293

Merged
merged 3 commits into from
Jun 1, 2022
Merged

Get sourcemaps working #4293

merged 3 commits into from
Jun 1, 2022

Conversation

slimbuck
Copy link
Member

@slimbuck slimbuck commented Jun 1, 2022

Fixes: #3996

This PR updates the engine rollup build script as follows:

  • remove rollup-replace plugin for setting module version and revision, use jscc for this instead
  • disable tabsToSpaces and shaderChunks processing in debug build, which appears to break sourcemaps (presumably because sourcemaps aren't returned from these functions)

With these changes in place sourcemaps work correctly in Chrome.

@slimbuck slimbuck requested a review from a team June 1, 2022 14:40
@slimbuck slimbuck self-assigned this Jun 1, 2022
@slimbuck slimbuck merged commit 78d003d into playcanvas:main Jun 1, 2022
@slimbuck slimbuck deleted the sourcemaps branch June 1, 2022 14:51
slimbuck added a commit that referenced this pull request Jun 1, 2022
* get sourcemaps working
* remove replace plugin dependency
* lint
Comment on lines +209 to +212
...{
_DEBUG: 1,
_PROFILER: 1
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks a bit verbose/strange, why not just add the properties directly? Saves two lines and indentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really sure what you mean, could you give an example?

Comment on lines +223 to +225
...{
_PROFILER: 1
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, this should be a simple property?

@kungfooman
Copy link
Collaborator

GitHub is acting strange for some reason, first the comments doubled and now they are still pending:

image

I will just write it once more and try to explain it here.

Code like this:

o = {
  ...{
    ...{
      ...{
        ...{
          ...{
            ...{
              ...{
                ...{
                    a: 123
                }
              }
            }
          }
        }
      }
    }
  }
}

Is basically the same as: o = {a: 123} e.g.

engine/rollup.config.js

Lines 205 to 216 in bf628f2

const jsccOptions = {
debug: {
values: {
...sdkVersion,
...{
_DEBUG: 1,
_PROFILER: 1
}
},
keepLines: true,
sourcemap: 'inline'
},

Line 215: sourcemap is a Boolean and not a string and by default true, so this can be completely removed:

https://github.com/aMarCruz/jscc/blob/7f8634b9677097c0e16311caae27937ff69d7f51/index.d.ts#L86-L93

@slimbuck
Copy link
Member Author

slimbuck commented Jun 8, 2022

Hi @kungfooman ,

Sourcemaps true will create a separate .map file. Sourcemaps inline will place them in the output module.

As for the destructuring assignment, you may have missed that _PROFILE etc is being combined with sdkVersion object. I didn't want to duplicate the version members multiple times.

This was the most straightforward method I could think of, but agree it's still rather messy.

@slimbuck
Copy link
Member Author

slimbuck commented Jun 8, 2022

(Oh and jscc declares the variable a boolean, but says to match it to the output sourcemap value, which is inline. I chose to keep inline because it's a little more explicit and works).

@kungfooman
Copy link
Collaborator

Hi @slimbuck thank you for your quick answer, I made a little example to demonstrate what I mean:

const version = 'version 1';
const revision = 'revision 2';
const sdkVersion = {
  _CURRENT_SDK_VERSION: version,
  _CURRENT_SDK_REVISION: revision
};
const values1 = { 
  ...sdkVersion, 
  _DEBUG: 1, 
  _PROFILER: 1
}
const values2 = { 
  ...sdkVersion,
  ...{
    _DEBUG: 1, 
    _PROFILER: 1
  }
}
console.log(values1, values2);

values1 and values2 are equal and still combined with sdkVersion, while values1 is using two lines less.

Sourcemaps true will create a separate .map file. Sourcemaps inline will place them in the output module.

You are defining inline sourcemaps in the jscc options, which is the preprocessor only? There is another inline option for the rollup targets (outputOptions), which I think you refer to.

@slimbuck
Copy link
Member Author

slimbuck commented Jun 8, 2022

OIC what you mean, yeah that's definitely neater. As for jscc inline, we could probably get rid of it.

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.

Sourcemaps debugging map seems to be off by 1 line in Chrome Devtools
4 participants