-
Notifications
You must be signed in to change notification settings - Fork 68
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
[RFC] 00010 UI5 Builder-Bundling Refactoring #583
Conversation
…code at the end of a bundle
Similar to Array.filter
b1ed587
to
6c6f066
Compare
|
Resolves SAP/ui5-tooling#472 Supersedes #282 Based on SAP/ui5-tooling#583 JIRA: CPOUI5FOUNDATION-434 Co-authored-by: Matthias Osswald <mat.osswald@sap.com>
@@ -158,8 +158,6 @@ To be discussed. | |||
## Unresolved Questions and Bikeshedding | |||
*This section should be removed (i.e. resolved) before merging* | |||
|
|||
Should we generate source maps during bundling if none is provided for a resource? This could be a simple mapping of only the first column of the first line to the original file. | |||
|
|||
### resourceRoot-mappings | |||
|
|||
When bundling modules via resourceRoot-mappings, it might become impossible to reference the original source from a bundle's source map. For example, for a bundle located in `/test-resources`, which includes modules from `/resources`, the source map must not reference the original module via `../../resources/x/y/z` as this path rather depends on the resourceRoot-mapping and might be different at runtime. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Line 172 IMO should be moved into the "Source Map support" section, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
The outdated annotation convention `//@` should not be supported. All known relevant tools (Terser, TypeScript compiler) are using the new convention. See [Conventions](http://sourcemaps.info/spec.html#h.lmz475t4mvbx) in the source map specification for details. | ||
|
||
Any source maps found for a resource shall be added to the source map of the bundle. Either use an [index map](http://sourcemaps.info/spec.html#h.535es3xeprgt), containing the individual source maps, or generate a new source map for the whole bundle based off them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's "based of them", isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
|
||
<!-- | ||
This is the bulk of the RFC. Explain the design in enough detail for somebody familiar with the UI5 Tooling to understand, and for somebody familiar with the implementation to implement. This should get into specifics and corner-cases, and include examples of how the feature is used. Any new terminology should be defined here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this comment be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There also further comments below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
## Drawbacks | ||
|
||
To be discussed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we have none, could we write it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are. I've added them.
|
||
## Alternatives | ||
|
||
To be discussed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we have none, could we write it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, too.
Read it here: RFC 0010 UI5 Builder-Bundling Refactoring
PoC: SAP/ui5-builder#657