Skip to content

New mappings structure#37

Merged
rwjblue merged 11 commits intoember-cli:masterfrom
Turbo87:new-structure
Oct 3, 2017
Merged

New mappings structure#37
rwjblue merged 11 commits intoember-cli:masterfrom
Turbo87:new-structure

Conversation

@Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Sep 18, 2017

This PR converts the mappings to the new structure proposed in #36 (comment)

We should only merge this once we've verified that the three main projects using this package can work with the new structure.

/cc @Serabe @locks

@rwjblue
Copy link
Member

rwjblue commented Sep 18, 2017

We should only merge this once we've verified that the three main projects using this package can work with the new structure.

You mean to confirm that it should be possible? Or that the work is actually done? The reason I am clarifying is that it is somewhat annoying to have to do the development with everything locally linked. In other words, I'm hoping that you mean to confirm that the individual project requirements can be satisfied by the new format...

@Serabe
Copy link
Contributor

Serabe commented Sep 18, 2017

@rwjblue the same information is still there.

For named exports:

// Old format
{
  // Some other fields
  "computed.gt": ["@ember/object/computed", "gt"]
}

// New format

[
  // Some other objects
  {
     "global": "Ember.computed.gt", // same as `Ember.${keyOldFormat}`
     "module": "@ember/object/computed", // same as valueOldFormat[0]
     "export": "gt", // same as valueOldFormat[1]
     "deprecated": false // no option in old format
  },
]

For default exports:

// Old format

{
  // Some other fields
  "Router": ["@ember/routing/router", null, "EmberRouter"],
}

// New format

[
  {
    "global": "Ember.Router", // same as `Ember.${keyOldFormat}`
    "module": "@ember/routing/router", // same as valueOldFormat[0]
    "export": "default",
    "localName": "EmberRouter", // same as valueOldFormat[2]
    "deprecated": false
  },
]

Which are all the projects depending on this one? I'm working on updating the babel plugin, so I can work on the others too.

@Serabe
Copy link
Contributor

Serabe commented Sep 18, 2017

@Turbo87 can change main in package.json to point to the new mapping file?

@rwjblue
Copy link
Member

rwjblue commented Sep 18, 2017

Getting the list from npmjs.com:

  • babel-plugin-ember-modules-api-polyfill
  • ember-modules-codemod
  • eslint-plugin-ember

@Serabe
Copy link
Contributor

Serabe commented Sep 18, 2017

@Turbo87 great work!

Serabe added a commit to Serabe/babel-plugin-ember-modules-api-polyfill that referenced this pull request Sep 18, 2017
This PR adapts the code to the new format suggested in
ember-cli/ember-rfc176-data#37.
Serabe added a commit to Serabe/eslint-plugin-ember that referenced this pull request Sep 18, 2017
This PR adapts the code to the new format in
ember-cli/ember-rfc176-data#37

This also removes a line testing an old shims not ported to new format.
Serabe added a commit to Serabe/ember-modules-codemod that referenced this pull request Sep 18, 2017
Serabe added a commit to Serabe/eslint-plugin-ember that referenced this pull request Sep 18, 2017
This PR adapts the code to the new format in
ember-cli/ember-rfc176-data#37

This also removes a line testing an old shims not ported to new format.
Serabe added a commit to Serabe/babel-plugin-ember-modules-api-polyfill that referenced this pull request Sep 18, 2017
This PR adapts the code to the new format suggested in
ember-cli/ember-rfc176-data#37.
@Serabe
Copy link
Contributor

Serabe commented Sep 18, 2017

PRs done:

All PRs bump version for ember-rfc176-data to ^0.3.0 (though I'm not a fan of ^, it was what it was) and assume main gets changed to the new json file. After this has been accepted, I'll just need to yarn to fix the yarn.lock files and that should be all.

Thank you!

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

LGTM, just needs the package.json main update that @Serabe mentioned.

@rwjblue
Copy link
Member

rwjblue commented Sep 19, 2017

After chatting with @Serabe in slack and in ember-cli/eslint-plugin-ember#142, it seems that we have lost some of the old shims in this conversion. For example, I do not see ember-controller/sortable in the new file, but I would expect it to be:

{ 
  global: 'Ember.Sortable', 
  module: 'ember-controller/sortable', 
  export: 'default', 
  deprecated: true 
}

@Turbo87
Copy link
Member Author

Turbo87 commented Sep 27, 2017

@rwjblue I've added the missing entries from the old shims now

@Serabe
Copy link
Contributor

Serabe commented Sep 29, 2017

Updated all three PRs

@rwjblue
Copy link
Member

rwjblue commented Sep 29, 2017

Just to be clear amongst us, we should make a few changes:

  • deprecated to either be a boolean or a string (which is a valid semver range). I don't believe that it really matters just yet (all of the current deprecated items are deprecated unrelated to specific versions), but it will matter for things like import { isHTMLSafe } from '@ember/string' which we plan to move from pointing to Ember.String.isHTMLSafe to Ember.Template.isHTMLSafe (and import { isHTMLSafe } from '@ember/template').
  • add an optional since field which is a semver range indicating when the import is valid from. Again, this isn't a blocker or anything, just a nice-to-have in the format for future use as new imports are made available to new versions.

@Serabe
Copy link
Contributor

Serabe commented Oct 3, 2017

What do we need to merge this and make a new release?

@rwjblue rwjblue merged commit dc31d12 into ember-cli:master Oct 3, 2017
@rwjblue
Copy link
Member

rwjblue commented Oct 3, 2017

Released as v0.3.0.

@Turbo87 Turbo87 deleted the new-structure branch October 3, 2017 15:09
rwjblue pushed a commit to Serabe/eslint-plugin-ember that referenced this pull request May 14, 2018
This PR adapts the code to the new format in
ember-cli/ember-rfc176-data#37

This also removes a line testing an old shims not ported to new format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants