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

[TIMOB-25768] Implement ability to use rollup #24

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

garymathews
Copy link
Contributor

  • rollup is required to workaround circular references unsupported by JavascriptCore
    • This should only be used on iOS and Windows platforms
    • Maybe we can make use of source-maps in the future?

JIRA Ticket

@janvennemann
Copy link
Contributor

Is this configurable and can be disabled together with transpilation? Titanium Angular uses TypeScript to generate compatible ES5 code and Webpack to bundle everything up, so none of would be required there.

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

I think we need to take a step back and start thinking through how this plus a number of other build operations will interact and how we could organize them in a robust way (and possibly allow user's to manipulate the set of transforms/plugins used). Or how we might be able to merge operations or avoid writing to disk and reading back/etc.

We now have:

  • Alloy doing some babel-based transformations from the app dir to Resources. We generate source maps for debugging purposes. (so we're reading, parsing, transforming, writing back to disk)
  • The SDK build then parsing each JS file and running some set of babel transforms/plugins on each file. (reading file, parsing, transforming, returning back to build where it's eventually written to disk. Here we do keep the original and modified source in-memory and avoid extra disk writes/reads.)
  • Not really explicit to all of this is the fact that we do some merging/filtering of the Resources folder based on platform, which results in paths possibly being altered. I have to assume this could throw off something like rollup unless we did a first-pass of merging/filtering.

We want to add in:

  • rollup (which could be hooked in as a babel plugin, btw)
  • angular (which it sounds like does it's own transforms/packing, so may need to skip some of this?)

@@ -171,6 +175,7 @@ exports.analyzeJs = function analyzeJs(contents, opts) {
// transpile
if (opts.transpile) {
options.presets.push([ env, { targets: opts.targets } ]);
options.plugins.push(require.resolve('@babel/plugin-transform-async-to-generator'));
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this plus the regenerator runtime are required to support async/await?


// generate code and a source map
// TODO: make use of source map
const { code, map } = await bundle.generate({
Copy link
Contributor

Choose a reason for hiding this comment

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

My concern here is that babel has generated bad source maps in the past for us, so I've been trying to work around the issue by using the retainLines: true option whenever we pass through babel.

Studio uses the source maps for debugging, but we've found out the hard way that babel's source map generation is not reliable. (We consume alloy's source maps)

It looks as though rollup actually uses acorn, not babel - so maybe it's more reliable?

We need to actually look for and combine the possible alloy source map into the process too.


// rollup is required to workaround circular references unsupported by JavascriptCore
// this should only run on 'app.js', bundling all dependencies into one file
if (opts.rollup) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the ability to include rollup in the pipeline is interesting and useful, but seems like a huge new addition here that we need to test/try out a bit before I'd feel too comfortable.

I assume the driver here was using ES6 imports with circular references on iOS?

Rollup should only be run on the single app entry point, and would be akin to minification in my mind.

I think we're quickly bumping into a large architectural issue here:

  • We need to find a way to be able to run some arbitrary set of transformations on the app source code
  • in a way that doesn't clash with one another
  • with the output of one transformation being the input to the next (i.e the modified source and source map).

@sgtcoolguy
Copy link
Contributor

sgtcoolguy commented Feb 14, 2018

On a related note - I think we could work towards collapsing some of the alloy babel plugins into the SDK itself...

Particularly this one: https://github.com/appcelerator/alloy/blob/master/Alloy/commands/compile/ast/optimizer-plugin.js

Basically it cheats and inlines the values of Ti.Platform.name and Ti.Platform.osname so that dead code elimination can prune unreachable branches.

I don't see why only Alloy would do that, it seems beneficial in general and possibly gives us a starting point to inline even more static values...

Possibilities:

  • Ti.Platform.architecture
  • Ti.Platform.ostype
  • Ti.Platform.runtime
  • Ti.buildHash
  • Ti.buildDate
  • Ti.version
  • Ti.App.analytics
  • Ti.App.copyright
  • Ti.App.deployType
  • Ti.App.description
  • Ti.App.guid
  • Ti.App.id
  • Ti.App.name
  • Ti.App.publisher
  • Ti.App.url
  • Ti.App.version
  • Ti.App.Properties usage (inline value on get calls. Flag sets as invalid if in tiapp.xml)
  • A plugin that strips Ti.API.log usage? (There's already some in babel that strip debugger calls or console.log calls)

@janvennemann
Copy link
Contributor

I totally agree with @sgtcoolguy. We are adding different build tools everywhere and should keep in mind how they interact with each other.

When i started working on Angular the Babel transpilation in our SDK was not there yet, so i used Rollup first and switched to Webpack later on, so another build tool is involved there too. It currently follows a similar approach to Alloy, as that the TypeScript sources are in the app folder and a project hook triggers webpack to compile and bundle everything up into the Resources dir where the SDK takes over like in a classic project.

@ffMathy
Copy link

ffMathy commented Dec 19, 2019

@garymathews this has conflicts. What is the status, and did you end up solving the issue? I'm having the issues you described here on SDK 8.5:

https://jira.appcelerator.org/browse/TIMOB-25768

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.

4 participants