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

fix: remove unnecessary inline SystemJS text plugin #51

Merged
merged 1 commit into from
Nov 15, 2018
Merged

fix: remove unnecessary inline SystemJS text plugin #51

merged 1 commit into from
Nov 15, 2018

Conversation

3cp
Copy link
Member

@3cp 3cp commented Nov 14, 2018

Both cli+bundler+systemjs and skeleton-navigation (jspm skeletons) bundle npm package systemjs-plugin-text. The previous inline text plugin in loader-default is unnecessary, and it prevents aurelia-i18n-loader from working properly with SystemJS.

closes aurelia/i18n#289

Both cli+bundler+systemjs and skeleton-navigation (jspm skeletons) bundle npm package systemjs-plugin-text. The previous inline text plugin in loader-default is unnecessary, and it prevents aurelia-i18n-loader from working properly with SystemJS.

closes aurelia/i18n#289
@3cp
Copy link
Member Author

3cp commented Nov 14, 2018

Tested with both mentioned setups.

cc @EisenbergEffect

@EisenbergEffect EisenbergEffect merged commit c95aa20 into aurelia:master Nov 15, 2018
@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented Nov 15, 2018

@huochunpeng Actually, just after I merged, I realized there might be a temporary problem with this.
Can you see if our getting started "todo" app tutorial works? There's a special build that currently relies on in the instructions. It uses system.js directly without JSPM. I'm thinking this may break that. We're going to move away from that but it's going to be another month probably before I can fully re-work that tutorial and build the in-browser sandbox. If you are able to confirm that that works, then I don't need to revert this and we can release. Sorry!

@3cp
Copy link
Member Author

3cp commented Nov 15, 2018

You are right, the todo app will break, but only after you pack a new aurelia-core.min.js for it.

Easy fix:
When you update aurelia-core.min.js for basic-aurelia-project,

  1. add a new file scripts/text.js

FYI, this is a better implementation of text plugin, copied from systemjs-plugin-text.

System.set('text', System.newModule({
  'translate': function (load) {
    load.metadata.format = 'amd';
    return 'define(function() {\nreturn ' + JSON.stringify(load.source) + ';\n});';
  }
}));
  1. update index.html to load text plugin after system.js.
<!DOCTYPE html>
<html>
  <head>
    <title>Aurelia</title>
  </head>
  <body aurelia-app="src/main">
    <script src="scripts/system.js"></script>
    <script src="scripts/text.js"></script>
    <script src="scripts/config-typescript.js"></script>
    <script src="scripts/aurelia-core.js"></script>
    <script>
      System.import('aurelia-bootstrapper');
    </script>
  </body>
</html>

@EisenbergEffect
Copy link
Contributor

We can't generate those files really anymore or re-pack them. We're killing them off entirely since we have an official script tag build. However, the tutorial doesn't match that either. So, we're in a bit of a sticky spot now.

@3cp
Copy link
Member Author

3cp commented Nov 15, 2018

If you don't re-pack them, it would not break.

@EisenbergEffect
Copy link
Contributor

LOL I'm silly. Thanks. I'll get this in, no problem. (embarrassed)

@3cp 3cp deleted the remove-inline-systemjs-text-plugin branch November 15, 2018 00:34
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.

The default backend loader doesn't work with cli+bundler+systemjs
2 participants