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

Examples: Deprecate examples/js scripts #17544

Closed
wants to merge 5 commits into from

Conversation

gkjohnson
Copy link
Collaborator

@gkjohnson gkjohnson commented Sep 20, 2019

When using a script from the examples/js/* directory a deprecation warning will be logged in the console (See #16920 (comment)):

THREE example script "js/controls/OrbitControls.js" is deprecated and will soon be removed. Use the ES module version instead: "jsm/controls/OrbitControls.js".

If this message feels right then I'll add it to all the files and update the PR.

edit: updated warning text, strip it in modularize.js

@donmccurdy
Copy link
Collaborator

/cc #16920 (comment)

@WestLangley
Copy link
Collaborator

.js is no longer maintained or updated.

We can't do that because the .js code, if included, must continue to work.

If you allow the js and jsm files to diverge, that will become a maintenance nightmare.

@gkjohnson
Copy link
Collaborator Author

@donmccurdy @WestLangley

We can't do that because the .js code, if included, must continue to work.

It's true that the js examples will likely break if they are not maintained or updated for a full year. From #16920 it seemed there was concern in converting new modules over to /js and giving the impression that we're still supporting them.

What if we meet in the middle and use rollup to only generate /js versions from modules for files that already exist in the /js folder and add a warning like the above to warn users that they will be deprecated. That script should be pretty simple, will keep them working until we want to remove them with no new modules being added, and will let us edit the jsm modules directly for PRs and testing. /js examples can be generated before a release like builds are.

@mrdoob thoughts?

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2019

/cc #16920 (comment)

@gkjohnson
Copy link
Collaborator Author

I've updated the text in the warning and added code for removing the warning from the file when generating the jsm files.

@@ -1,3 +1,5 @@
console.warn( 'THREE example script "js/controls/OrbitControls.js" is deprecated and will soon be removed. Use the ES module version instead: "jsm/controls/OrbitControls.js".' );
Copy link
Collaborator

@donmccurdy donmccurdy Sep 24, 2019

Choose a reason for hiding this comment

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

I would include the intended date of removal, to force feedback sooner than later, and perhaps reference an issue. Suggestion:

THREE.OrbitControls: Scripts in "examples/js/*" are deprecated and will be removed in January 2020 (r113). Use an ES module instead, "examples/jsm/controls/OrbitControls.js". See: https://github.com/mrdoob/three.js/issues/<TBD>.

^Let's open a new issue to track the change and help users resolve issues.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good I'll make the change -- @mrdoob it should probably be you who makes the issue to announce the intent to remove and then I can reference that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ping @mrdoob any plans to make an issue with a deprecation date?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mrdoob any updated plans on this?

Copy link
Collaborator

@Mugen87 Mugen87 Jan 28, 2020

Choose a reason for hiding this comment

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

AFAIK, no files depend in the repository on examples/js anymore. However, it would be good if an additional pair of eyes verifies this^^.

If there are no dependencies anymore, I think we should move on with the removal by adding this log to all js files.

Copy link
Collaborator

@Mugen87 Mugen87 Jan 28, 2020

Choose a reason for hiding this comment

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

We should also publish a small guide in the docs (maybe an additional section in Import via modules) which explains what users have to do if they used this style:

const THREE = window.THREE = require( 'three' );
require( 'three/examples/js/loaders/GLTFLoader.js' );

or a included the scripts directly:

<script src="https://unpkg.com/[email protected]/build/three.js"></script>
<script src="https://unpkg.com/[email protected]/examples/js/loaders/GLTFLoader.js"></script>

This would make support easier since we can always refer to the same guide.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 27, 2020

Closing in favor of #18749.

@Mugen87 Mugen87 closed this Feb 27, 2020
@gkjohnson gkjohnson deleted the example-warning branch February 27, 2020 16:08
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.

5 participants